Created attachment 91474 [details] proposed patch The poppler library outputs some warnings (PDF syntax warnings/error etc) to stderr by default. For evince, we want to suppress that output by default, so we need some poppler-glib API to wrap setErrorCallback from Error.h (we don't want to use poppler internal APIs directly in evince).
After discussing this on IRC, we came up with a different scheme where instead of creating new API, we just redirect the messages to the GLib logging framework. Since there's no single entry point in the public poppler-glib API, we just use a library constructor for this.
Created attachment 92383 [details] [review] new patch
Created attachment 92384 [details] [review] updated patch
Created attachment 92386 [details] [review] make the strings a bit more informative Patch tested with evince, works fine. By default, nothing is printed; run with G_MESSAGES_DEBUG=Poppler (or =all) to see the messages.
Comment on attachment 92384 [details] [review] updated patch I guess this one is obsolete now
Comment on attachment 92386 [details] [review] make the strings a bit more informative Review of attachment 92386 [details] [review]: ----------------------------------------------------------------- Yes, a lot simpler, and we don't need new API nor even change evince. ::: glib/poppler.cc @@ +90,5 @@ > + category == errCommandLine || > + category == errNotAllowed) > + return; > + > + g_log (G_LOG_DOMAIN, G_LOG_LEVEL_INFO, Shouldn't we use "Poppler" as log domain? @@ +95,5 @@ > + "%s at position %" G_GOFFSET_FORMAT ": %s", > + cat_str[category], (goffset) pos, message); > +} > + > +static void poppler_constructor (void) __attribute__((__constructor__)); Why do you need this prototype? I think you can annotate the method directly, for example: static void __attribute__((__constructor__)) poppler_constructor (void) { setErrorCallback (error_cb, NULL); }
Created attachment 92409 [details] [review] updated patch (In reply to comment #6) > > + g_log (G_LOG_DOMAIN, G_LOG_LEVEL_INFO, > > Shouldn't we use "Poppler" as log domain? It does; G_LOG_DOMAIN is #define'd to "Poppler" in Makefile.am (INCLUDES). > > +static void poppler_constructor (void) __attribute__((__constructor__)); > > Why do you need this prototype? I think you can annotate the method > directly, for example: > > static void __attribute__((__constructor__)) poppler_constructor (void) > { > setErrorCallback (error_cb, NULL); > } Fixed.
(In reply to comment #7) > Created attachment 92409 [details] [review] [review] > updated patch > > (In reply to comment #6) > > > + g_log (G_LOG_DOMAIN, G_LOG_LEVEL_INFO, > > > > Shouldn't we use "Poppler" as log domain? > > It does; G_LOG_DOMAIN is #define'd to "Poppler" in Makefile.am (INCLUDES). Oh, right! > > > +static void poppler_constructor (void) __attribute__((__constructor__)); > > > > Why do you need this prototype? I think you can annotate the method > > directly, for example: > > > > static void __attribute__((__constructor__)) poppler_constructor (void) > > { > > setErrorCallback (error_cb, NULL); > > } > > Fixed. Pushed to git master, thank you very much.
*** Bug 57131 has been marked as a duplicate of this bug. ***
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.