After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679116 - Output debug message if modules are not loaded because of mime type detection
Output debug message if modules are not loaded because of mime type detection
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-29 09:49 UTC by Guillaume Desmottes
Modified: 2012-07-06 08:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backend-store: output a debug message if mime type of potential backends looks weird (1.51 KB, patch)
2012-06-29 09:55 UTC, Guillaume Desmottes
needs-work Details | Review
backend-store: output a debug message if mime type of potential backends looks weird (1.35 KB, patch)
2012-07-02 07:00 UTC, Guillaume Desmottes
committed Details | Review
backend-store: never translate warning messages (2.64 KB, patch)
2012-07-02 07:02 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2012-06-29 09:49:15 UTC
I wasted another hour of my life trying to understand why Folks was not working with my test user.

Turned out it was because shared-mime-info which is a problem I already hit in the past (bug #626108). By then we added some debugging if the mime code was failing to guess the type. But it looks like it's not a bit smarter and assume it's 'text/plain' as a wild guess.

I'd like to add some debugging in this case so I won't waste time on this any more next time I'll clean my jhbuild usr :)
Comment 1 Guillaume Desmottes 2012-06-29 09:55:54 UTC
Created attachment 217595 [details] [review]
backend-store: output a debug message if mime type of potential backends looks weird

Will make my life much easier next time I'm wondering why nothing is working.
Comment 2 Jeremy Whiting 2012-06-29 15:30:00 UTC
Review of attachment 217595 [details] [review]:

Looks good to me, go for it.
Comment 3 Philip Withnall 2012-07-01 23:23:08 UTC
Review of attachment 217595 [details] [review]:

::: folks/backend-store.vala
@@ +644,3 @@
+                  /* Translators: the first parameter is a filename and the
+                   * second a mime type like 'text/plain'. */
+                  _("The content type of '%s' appears to be '%s' which looks suspicious. Have you installed shared-mime-info?"), file.get_path (), mime);

Debug messages shouldn’t be translated. I guess this should probably be a warning() rather than a debug().
Comment 4 Philip Withnall 2012-07-01 23:23:52 UTC
Might also be worth making shared-mime-info a hard dependency of jhbuild’s bootstrap, because not having it installed in a jhbuild prefix seems to cause no end of problems.
Comment 5 Guillaume Desmottes 2012-07-02 07:00:16 UTC
Created attachment 217803 [details] [review]
backend-store: output a debug message if mime type of potential backends looks weird

Will make my life much easier next time I'm wondering why nothing is working.
Comment 6 Guillaume Desmottes 2012-07-02 07:02:09 UTC
Created attachment 217804 [details] [review]
backend-store: never translate warning messages

Most of them were not translated.
Comment 7 Guillaume Desmottes 2012-07-02 07:03:38 UTC
(In reply to comment #4)
> Might also be worth making shared-mime-info a hard dependency of jhbuild’s
> bootstrap, because not having it installed in a jhbuild prefix seems to cause
> no end of problems.

That wouldn't help me tbh, I never use jhbuild bootstrap. I use as much distro component as possible and just build the bits I really need.
Comment 8 Philip Withnall 2012-07-02 20:36:48 UTC
Review of attachment 217803 [details] [review]:

Looks good to me.
Comment 9 Philip Withnall 2012-07-02 20:37:38 UTC
Review of attachment 217804 [details] [review]:

What’s the reasoning behind this? These are potentially user-visible strings.
Comment 10 Philip Withnall 2012-07-02 20:44:08 UTC
(In reply to comment #7)
> (In reply to comment #4)
> > Might also be worth making shared-mime-info a hard dependency of jhbuild’s
> > bootstrap, because not having it installed in a jhbuild prefix seems to cause
> > no end of problems.
> 
> That wouldn't help me tbh, I never use jhbuild bootstrap. I use as much distro
> component as possible and just build the bits I really need.

I’ve added shared-mime-info as a hard dependency of folks in jhbuild. That should fix things.
Comment 11 Guillaume Desmottes 2012-07-03 07:38:50 UTC
(In reply to comment #9)
> Review of attachment 217804 [details] [review]:
> 
> What’s the reasoning behind this? These are potentially user-visible strings.

Really? Aren't warning() just displayed in stderr and so not presented in the UI? You said said in Comment 3 than those should not be translated.
Comment 12 Philip Withnall 2012-07-03 20:24:45 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > Review of attachment 217804 [details] [review] [details]:
> > 
> > What’s the reasoning behind this? These are potentially user-visible strings.
> 
> Really? Aren't warning() just displayed in stderr and so not presented in the
> UI? You said said in Comment 3 than those should not be translated.

warning() are just displayed in stderr, but they end up in .xsession-errors and I reckon they could be useful to users for debugging problems.

In comment #3 I said that debug() messages shouldn’t be translated. I’d be open to arguments as to why warning() messages also shouldn’t be translated. :-)
Comment 13 Guillaume Desmottes 2012-07-04 07:31:47 UTC
Only strings ending up in the UI are supposed to be translated. If an user is good enough to try understanding an issue in .xession-errors he will probably understand it in English anyway.

Furthemore, translated warnings won't be that useful to us when we'll receive them in chinese in a bug report. :)
Comment 14 Bastien Nocera 2012-07-04 11:43:06 UTC
(In reply to comment #13)
> Only strings ending up in the UI are supposed to be translated. If an user is
> good enough to try understanding an issue in .xession-errors he will probably
> understand it in English anyway.
> 
> Furthemore, translated warnings won't be that useful to us when we'll receive
> them in chinese in a bug report. :)

Seconded. g_warning() output shouldn't be translated.

But I would switch that code to be a self-test instead, when libfolks gets initialised.

Call g_content_type_guess() on a (short) piece of data that you know should be recognised as a particular type, and warn in that case.
Comment 15 Philip Withnall 2012-07-05 19:24:36 UTC
Review of attachment 217804 [details] [review]:

Make it so, then!
Comment 16 Philip Withnall 2012-07-05 19:35:24 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > Only strings ending up in the UI are supposed to be translated. If an user is
> > good enough to try understanding an issue in .xession-errors he will probably
> > understand it in English anyway.
> > 
> > Furthemore, translated warnings won't be that useful to us when we'll receive
> > them in chinese in a bug report. :)
> 
> Seconded. g_warning() output shouldn't be translated.

Filed bug #679467 about it, because I keep forgetting what people tell me about this.

> Call g_content_type_guess() on a (short) piece of data that you know should be
> recognised as a particular type, and warn in that case.

Guillaume, if you want to re-do the patch to do this, feel free. I’m not entirely sure it’s necessary; unless I’ve missed something it just means the user will get one warning instead of one per backend.
Comment 17 Guillaume Desmottes 2012-07-06 08:02:57 UTC
Attachment 217803 [details] pushed as 8034c35 - backend-store: output a debug message if mime type of potential backends looks weird
Attachment 217804 [details] pushed as b67a111 - backend-store: never translate warning messages