Support here is *optional*, as in it does *not* require even the GIR M4 macros to be available -- but then the generated build system can't generate GIR. This avoids a hard dependency on GIR for the cost of a configure-time conditional with basic fallback definitions.
As stated in the first commit, it's mostly ripped off Peasy, maybe there are additional change required, but it seems to mostly work. However, I see a lot of scary warnings like these: ``` …/doc/geany-sciwrappers-gtkdoc-tmp.h:25: syntax error, unexpected '*', expecting ')' or ',' in 'void scintilla_object__GI__MARK_set_text (ScintillaObject *sci, const gchar *text);' at '*' …/doc/geany-sciwrappers-gtkdoc-tmp.h:25: syntax error, unexpected ')', expecting ',' or ';' in 'void scintilla_object__GI__MARK_set_text (ScintillaObject *sci, const gchar *text);' at ')' …/doc/geany-sciwrappers-gtkdoc-tmp.h:43: syntax error, unexpected '*', expecting ')' or ',' in 'void scintilla_object__GI__MARK_start_undo_action (ScintillaObject *sci);' at '*' …/doc/geany-sciwrappers-gtkdoc-tmp.h:64: syntax error, unexpected '*', expecting ')' or ',' in 'void scintilla_object__GI__MARK_end_undo_action (ScintillaObject *sci);' at '*' …/doc/geany-sciwrappers-gtkdoc-tmp.h:84: syntax error, unexpected '*', expecting ')' or ',' in 'void scintilla_object__GI__MARK_set_marker_at_line (ScintillaObject *sci, gint line_number, gint marker);' at '*' ```
@codebrainz @kugel- You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1112
-- Commit Summary --
* Add support for building GIR * Force enabling GObject-Introspection for distcheck
-- File Changes --
M Makefile.am (2) M configure.ac (12) M doc/Makefile.am (69)
-- Patch Links --
https://github.com/geany/geany/pull/1112.patch https://github.com/geany/geany/pull/1112.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
What is the meaning of `pre` used all over here?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
IIUC, intermediate files that get fixed afterward. I guess `tmp` might make more sense
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
I see. `tmp` does make a bit more sense and is already used for the header.
But why not just output the correct text to begin with rather than patching it up afterwards using intermediate files?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
For the sciwrappers part, because g-ir-scanner can't be told to put method prefixed with `sci` into the `ScintillaObject` namespace, so have nice API (i.e. `sci.start_undo_action()`) the code here renames the functions to a `scinitilla_object` prefix and strip them back to have the proper name. Yep, it's a hack.
Not sure about the `GeanyScintillaScintillaObject` thing.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
This is so that `Geany.sci_start_undo_action(sci)` works at the same time (on the same object instance) as GeanyScintilla's `sci.start_undo_action()`.
For this to work, `Geany.sci_start_undo_action()` must take a GeanyScintilla.ScintillaObject parameter (same type as `sci.start_undo_action()`)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
However, I see a lot of scary warnings like these:
It's a bit cryptic, buit these usually indicate that g-ir-scanner doesn't know about a type which is used, e.g. when the typedef isn't provided (either in the same file or via #include or --c-include). In this case ScintillaObject appears to be unknown.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230246588
@b4n pushed 3 commits.
80cce4e Properly pass -DGTK when building GIR 2acd78a Rename temporary intermediate GIR files using tmp suffix for clarity d3fdcc9 Move common GIR CFLAGS to a reusable variable and add missing ones
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1112/files/dbd702d8bec73ae9a5adea857a857...
@b4n pushed 1 commit.
800f46d Fix GIR scanning order of Scintilla headers
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1112/files/d3fdcc921c61ff567cfb33e5ab6f4...
Do you intend to merge this or is this just in response to @codebrainz here https://github.com/geany/geany/pull/1094#issuecomment-229753093
I support this, it was my original wish that Geany ships the .gir and .typelib (possibly even the .vapi).
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230285061
It's a bit cryptic, buit these usually indicate that g-ir-scanner doesn't know about a type which is used, e.g. when the typedef isn't provided (either in the same file or via #include or --c-include). In this case ScintillaObject appears to be unknown.
Thanks, that was missing `-DGTK`, fixed now.
However, I also got this one:
``` …scintilla/include/ScintillaWidget.h:42: syntax error, unexpected identifier in ' void (* notify) (ScintillaObject *sci, int id, SCNotification *scn);' at 'SCNotification' ```
I finally found that it's because *ScintillaWidget.h* doesn't include *Scintilla.h* that defines `SCNotification`. Reordering the parsing order fixes the issue. But your Peasy has the wrong order, so do you get that issue too?
Also, I dropped ``` SCANNERFLAGS += --program-arg=--introspection-dump=$(srcdir)/get-type.txt,$(builddir)/get-type.out ``` which I didn't get the point of, and that didn't seem necessary. And your *api/get-type.txt* file lists an internal function `tm_tag_get_type()` so it seemed odd.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230285396
Do you intend to merge this or is this just in response to @codebrainz here #1094 (comment)
I indented this to be a response to @codebrainz's point, but also try and get something non-intrusive and mergeable. So yeah, when it gets polished a little I'd be willing to merge it, so long as it works.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230285778
@@ -148,6 +148,75 @@ CLEAN_LOCAL_TARGETS += clean-gtkdoc-header-local clean-gtkdoc-header-local: -rm -rf xml/ Doxyfile-gi Doxyfile-gi.stamp geany-gtkdoc.h geany-sciwrappers-gtkdoc.h
+if HAVE_INTROSPECTION
+INTROSPECTION_GIRS = Geany-1.0-pre.gir GeanyScintilla-1.0-pre.gir
Why do we need both? The point of the former is to provide a method on the Scintilla object (well as much as C can), if the latter does that, then why have both?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/a1751ae651425b7443f4fd3319b8b...
@@ -196,12 +196,12 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
-GeanyScintilla-1.0.gir: GeanyScintilla-1.0-pre.gir +GeanyScintilla-1.0.gir: GeanyScintilla-1.0-tmp.gir $(AM_V_GEN) cat $< \ | $(SED) -e 's,scintilla_object__GI__MARK_,sci_,g' \
What does the `_GI__MARK_` do? I couldn't find anywhere (ex. in the codegen script) that introduces it. Is it actually needed or left over from some other name-munging code that was removed?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/80cce4ec54e6778b1a25efe79cd4d...
@@ -196,12 +196,12 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
-GeanyScintilla-1.0.gir: GeanyScintilla-1.0-pre.gir +GeanyScintilla-1.0.gir: GeanyScintilla-1.0-tmp.gir $(AM_V_GEN) cat $< \
I think I already know the answer, but just in case, why not pipe `g-ir-scanner`'s output directly through `sed`, bypassing any intermediate files? It writes to stdout by default.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/80cce4ec54e6778b1a25efe79cd4d...
@@ -196,12 +196,12 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
-GeanyScintilla-1.0.gir: GeanyScintilla-1.0-pre.gir +GeanyScintilla-1.0.gir: GeanyScintilla-1.0-tmp.gir $(AM_V_GEN) cat $< \ | $(SED) -e 's,scintilla_object__GI__MARK_,sci_,g' \
it's a custom mark first added to *geany-sciwrappers-gtkdoc-tmp.h* so we can easily catch the methods we need to rename, without messing up with get_type or others like that.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/80cce4ec54e6778b1a25efe79cd4d...
@@ -196,12 +196,12 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
-GeanyScintilla-1.0.gir: GeanyScintilla-1.0-pre.gir +GeanyScintilla-1.0.gir: GeanyScintilla-1.0-tmp.gir $(AM_V_GEN) cat $< \
@kugel- would need to answer, but I'd guess it's because `g-ir-scanner` is called by the generic introspection makefile rather than implemented manually, so we can't easily sed its output. If we can, that'd be nicer indeed.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/80cce4ec54e6778b1a25efe79cd4d...
@@ -196,12 +196,12 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
-GeanyScintilla-1.0.gir: GeanyScintilla-1.0-pre.gir +GeanyScintilla-1.0.gir: GeanyScintilla-1.0-tmp.gir $(AM_V_GEN) cat $< \ | $(SED) -e 's,scintilla_object__GI__MARK_,sci_,g' \
Oh, I see, it's added right here in this file and then removed again. I was looking in the Python script for where this mark is printed.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/80cce4ec54e6778b1a25efe79cd4d...
or is this just in response to @codebrainz here #1094 (comment)
I assume also in response to #1104
when it gets polished a little I'd be willing to merge it, so long as it works.
IMO, it shouldn't be `dist`ed through Autotools yet though, at least not without having some kind of way to tell users it's experimental and expected to change in incompatible ways. The main thing for now is that we be able to test and try various stuff for PRs, IMO.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230332522
I added https://github.com/b4n/geany/pull/2 (reminder for future-self).
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230366489
IMO, it shouldn't be `dist`ed through Autotools yet though
you mean installed I guess? (`dist` is in tarballs)
at least not without having some kind of way to tell users it's experimental and expected to change in incompatible ways. The main thing for now is that we be able to test and try various stuff for PRs, IMO.
GIRs include a version, and it's in their name. We could also just change this version whenever we make incompatible changes (kinda like with soname in theory).
But yeah, as it's not yet very much used, we can't know all the problems in it, so we need to be able to fix it without upsetting users too much. So yeah, make them sign the "this is beta, you've been warned" contract :)
First interesting thing would be to know if this GIR is nicely usable directly by @kugel-'s Peasy and plugins, so that we at least didn't regress from that.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230466715
I did have the .gir installed by Geany for peasy initially (until you suggested you aren't going to like it) so it should work. I'll check again to make sure.
But if it's not installed it's not gonna be useful for peasy. IMO it should be sufficient to warn in the release notes if it's experimental. Actually, I too don't want to miss the ability to make breaking changes as long as the upbringing is still ongoing. But of course, later on I'd like some kind of stability promise, @gironly shouldn't mean "we can break it any time" forever.
I don't think anyone else is using the gtkdoc header or .gir yet.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230468066
IMO it should be sufficient to warn in the release notes if it's experimental.
Actually should also be noted in README so it shows on the front of github and in the docs that tell you about using GIR (you do have those?) in big flashing orange and purple letters.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230468949
@b4n pushed 2 commits.
d8a756c gir: generate better output for `document_new_file` 194dd04 Don't needlessly chain sed calls
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1112/files/800f46d15ae2d52eb43145b74a19d...
I don't think anyone else is using the gtkdoc header or .gir yet.
BTW, if we provide GIR, do we need continuing distributing gtkdoc headers? Seems redundant to me, ain't it?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230476356
BTW, if we provide GIR, do we need continuing distributing gtkdoc headers? Seems redundant to me, ain't it?
I don't need them for anything else. Feel free to drop un-install them.
I pushed a temporary brach to peasy which uses the .gir shipped by Geany (with this PR): https://github.com/kugel-/peasy/tree/gir-from-geany
Seems to work fine. I also checked that the .vapi still generates correctly (i.e. not a constructor) and that's the case.
So yea, I'm happy if this PR lands (maybe even for 1.28?) :+1:
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-230488947
@@ -194,22 +194,29 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
+geany-gtkdoc-tmp.h: geany-gtkdoc.h
- $(AM_V_GEN) $(SED) \
-e 's,\(document_\)\(new_file\),\1_GI__WORKAROUNDNEW\2,g' \
I haven't tested this name yet, but the reason I chose the name in my PR was to ensure that `g-ir-scanner` doesn't get confused by `NEW` in the name or somehow skipping `UPPERCASE` names or something. I don't trust that tool :) If it works though, good stuff.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/800f46d15ae2d52eb43145b74a19d...
@@ -194,22 +194,29 @@ geany-sciwrappers-gtkdoc-tmp.h: geany-sciwrappers-gtkdoc.h -e 's,sci_,scintilla_object__GI__MARK_,g' \ $< > $@
+geany-gtkdoc-tmp.h: geany-gtkdoc.h
- $(AM_V_GEN) $(SED) \
-e 's,\(document_\)\(new_file\),\1_GI__WORKAROUNDNEW\2,g' \
Yeah I wasn't 100% sure but I felt the GIR devs couldn't be that careless, so I went ahead and tried, and it worked fine, so I chose that one because it's more obvious and easier to strip.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112/files/800f46d15ae2d52eb43145b74a19d...
We should probably remove the `--enable-gtkdoc-header` option since it's effectively rolled in to `--enable-introspection` after this PR.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-232235961
@codebrainz yes, I know and plan to do that at some point, but it might be kinda subtle to do because the `--enable-introspection` is added automatically. But there's probably some nice way to do that anyway, just needs some work.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-232356812
because the --enable-introspection is added automatically
@b4n I thought --enable-introspection [was added manually](https://github.com/geany/geany/pull/1112/commits/a1751ae651425b7443f4fd3319b...). Or did I misunderstand you?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-232516599
@codebrainz No, your link points to the stub fallback implementation for when `GOBJECT_INTROSPECTION_CHECK` is unavailable (at autogen time).
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-232518292
@b4n Is it the case that if the user has the required deps, the GIR stuff will be built, if not, it will fail gracefully? If so, that seems OK, as long as the user can put `--disable-introspection` to disable the GIR stuff even if they have the deps. (Sorry if I'm misunderstanding the problem still).
On a related matter, since the GTK-DOC/GIR/codegen contraption requires Doxygen, should `--disable-introspection` be implied if Doxygen isn't found or if `--disable-api-docs` is passed (if that's not currently the case in this PR)?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1112#issuecomment-232519444
@b4n @codebrainz what's missing here?
@b4n @codebrainz ping
@b4n @codebrainz ping
@b4n @codebrainz ping
@b4n @codebrainz what's missing here?
At the least @b4n needs to address @codebrainz last point of this checking the dependency on doxygen?
@kugel- It's missing proper handling of the configure options; that is that introspection gets disabled automatically if one of the other deps is missing, and that force-enabling it properly checks the dependencies are there. I'll try and get back to this, but it might be subtle as the introspection enabling flag is not done by us but my the GIR macro.
What dependencies do you mean? Tools like g-ir-scanner seem to be implicit, as they are part of the gobject-introspection-1.0 package (as in pkg-config package).
I opened https://github.com/b4n/geany/pull/3, hopefully we're golden now
@b4n ping
@b4n ping
@b4n ping
@b4n ping
github-comments@lists.geany.org