GNOME Bugzilla – Bug 690774
Various improvements
Last modified: 2012-12-28 21:41:37 UTC
Another set of patches with various improvements.
Created attachment 232271 [details] [review] GTK-Doc: gdk-pixbuf was missing in FIXXREF_OPTIONS
Created attachment 232272 [details] [review] Use G_GNUC_INTERNAL to avoid exporting private symbols With the relinfo.pl script from Ulrich Drepper, here are the results. Before: 128 relocations, 105 relative (82%), 946 PLT entries, 247 for local syms (26%) After: 125 relocations, 103 relative (82%), 880 PLT entries, 181 for local syms (20%) Private symbols prefixed with an underscore (or without the gtk_source_ prefix) were not exported. However G_GNUC_INTERNAL is beneficial for them too because the compiler is now aware that those symbols are not exported, and therefore it can better optimize the code. Another problem remains: the big number of PLT entries for local symbols. Normally it should be 0 I think (it is 0 for the glib, gio and gtk+, for example, so it should be possible for GSV too).
Created attachment 232273 [details] [review] Use the GtkSourceView type instead of "struct _GtkSourceView"
Created attachment 232274 [details] [review] Protect completionwordsutils.h from several inclusions
Created attachment 232275 [details] [review] Include the completion providers in gtksource.h
Created attachment 232276 [details] [review] tests: clean header inclusions
Created attachment 232277 [details] [review] Remove unused symbols The symbols were present in docs/reference/gtksourceview-3.0-unused.txt, but they are used nowhere, so it's simpler to remove them.
Created attachment 232278 [details] [review] Document completion error symbols GTK_SOURCE_COMPLETION_ERROR_ALREADY_BOUND is used only in gtk_source_completion_add_provider(). GTK_SOURCE_COMPLETION_ERROR_NOT_BOUND is used only in gtk_source_completion_remove_provider().
Created attachment 232279 [details] [review] Move some private types in gtksourcetypes-private.h Some private types were present in gtksourcetypes.h. Another solution would have been to move these types in their respective headers, but we lose the benefit of having one header with all the types (to avoid header inclusions cycles). The lists of headers in the Makefile.am files have been sorted, and gtksourcetypes-private.h has been added.
Created attachment 232280 [details] [review] GTK-Doc: add missing symbols Now the gtksourceview-3.0-unused.txt file is empty.
Review of attachment 232271 [details] [review]: thanks!
Review of attachment 232272 [details] [review]: A bit unfortunate that we have to annotate everything, but oh well, as far as I can see gtk did the same
Review of attachment 232273 [details] [review]: definitely good
Review of attachment 232274 [details] [review]: Ouch. Good catch!
Review of attachment 232275 [details] [review]: I am not convinced about this one... what's the gain?
Review of attachment 232276 [details] [review]: Go for it (but you will need to readd the completionwords header, unless you convince me that there it should go in gtksource.h)
Review of attachment 232276 [details] [review]: I forgot to mark it...
Review of attachment 232277 [details] [review]: Definitely good
Review of attachment 232278 [details] [review]: great
Review of attachment 232279 [details] [review]: perfect
It looks like bugzilla is refusing to publish comments on the other patches... anyway to sum it up: they all look good except the one putting wordscompletion in gtksource.h which I am not really convinced about
OK, thanks for the review. I've pushed the good commits.