GNOME Bugzilla – Bug 679424
Fix some function's prototypes
Last modified: 2016-10-14 18:28:35 UTC
I think this doesn't break the API, since the same types are used, but just with another form.
Created attachment 218060 [details] [review] Fix some function's prototypes
Created attachment 221060 [details] [review] Redifining two times the same type with typedef is not allowed in C. So here is another solution, that places some typedef in the gtksourcecompletion-private.h file.
Created attachment 221061 [details] [review] Better parameter type for _gtk_source_completion_context_new() It is not strictly useful since the function is private, but since it's easy to do, why not.
Created attachment 221062 [details] [review] Direct include of completioncontext.h Instead of an indirect include via completionprovider.h.
Created attachment 221063 [details] [review] Better parameter type for gtk_source_completion_context_add_proposals() Here the cycle was between completionprovider.h and completioncontext.h.
Created attachment 221064 [details] [review] CompletionContext: reformat the prototypes
The patch series looks good, please push. Thanks!
Thank you for the review. The commits are pushed.
Reopening as gedit is not able to compile against gtksourceview anymore. This is due to the fact that gtksourceview-private shouldn't be in gtksourceview.h
Created attachment 221323 [details] [review] Fix inclusion of private .h in public headers The solution is to put the problematic typedefs in a new .h file, called gtksourcecompletiontypedefs.h. (Sorry for the inconvenience)
It's the kind of things that I don't pay attention to, as I don't have a lot of experience in library development. Maybe there is a better name for gtksourcecompletiontypedefs.h? I don't know how these issues are generally resolved. (or maybe they are not generally resolved ;) PS: last week and the following weeks I don't have an access to IRC (or I should learn irssi or something similar, but I'm not often connected to internet, anyway).
The way in which gtk solves it is by having an header for all types. I propose to byte the bullet and just do gtksourcetypes.h
Created attachment 221450 [details] [review] Fix inclusion of private .h in public headers The solution is to put the problematic typedefs in a new .h file, called gtksourcetypes.h, like it is done in GTK+. (Sorry for the inconvenience)
Well, I would like to move all the types there, not just completion :)
Created attachment 221656 [details] [review] Now it should be OK. I've moved all the main types (not the *Class or *Private types). But some .h files were included for having the type that is now in gtksourcetypes.h. So some headers clean-up should be done, ideally. But it takes far more time to do (and it is boring...), unless a script exists. ---- Move all the main types in gtksourcetypes.h This avoids the problem of .h inclusions cycles.
Review of attachment 221656 [details] [review]: Looks good! Please commit so that we unbreak things. We can worry about the include cleanup later
I committed this since it was kind of urgent
Created attachment 254511 [details] [review] 0001-SourceStyle-Use-the-expected-typedef-for-GtkSourceSt.patch This has created a slight build problem with gtksourceviewmm (C++). This patch fixes it. May I push it?
Review of attachment 254511 [details] [review]: Fair enough.
(In reply to Paolo Borelli from comment #12) > The way in which gtk solves it is by having an header for all types. > > I propose to byte the bullet and just do gtksourcetypes.h So gtksourcetypes.h has been created to avoid cycles in header inclusions, when header A needs the type B and header B needs the type A. At: https://wiki.gnome.org/Projects/GLib/CompilerRequirements > (C11) support for type redefinition > > /!\ This requirement has been temporarily suspended (on account of OpenBSD > carrying an old version of gcc) but it will probably return in the future. > > Your compiler must allow "a typedef name [to] be redefined to denote the same > type as it currently does", as per C11 §6.7, item 3. With that C11 feature, it is possible to solve the above problem by redefining the type. If GLib uses again this feature, we can consider using it in GtkSourceView too. Not sure it is something we want to do, but it's better to have self-contained *.{c,h}, instead of needing to look at other places. A self-contained *.{c,h} (or a group of related *.{c,h}) can then be copied in another project, or even forked (to quickly experiment something in an application or a higher-level library).