This is probably not ready yet, but open for review. The new script probably needs some more cleanup but since I'm not sure anyone is interested in reviewing ruby code I decided to PR'ify.
Description: This PR does some prep work, then enables generating xml output from doxygen, and lastly generate gtkdoc'ized headers from the xml output. The gtkdoc'ized headers in turn are input to g-ir-scanner and g-ir-compile but that's not port of the PR yet (but I have it working on my side).
If we decide to not ship the actual .gir and .typelib files for gobject-introspection yet, then we should at least ship the gtkdoc headers so that I can use it to generate .gir and .typelib on my own in my plugin for now.
You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/890
-- Commit Summary --
* doxygen: generate xml too in preparation for gtkdoc generation * doxygen: various doxygen-related fixes in preparation for gtkdoc generation * api: add script to generate {geany,geany-scintilla}-gtkdoc.h
-- File Changes --
M .gitignore (1) M configure.ac (1) M doc/Doxyfile.in (16) M doc/Makefile.am (40) M doc/pluginsymbols.c (4) M m4/geany-docutils.m4 (16) A scripts/gen-api-gtkdoc.rb (490) M src/document.c (18) M src/editor.c (2) M src/editor.h (2) M src/encodingsprivate.h (6) M src/filetypes.c (19) M src/filetypes.h (3) M src/keybindings.c (10) M src/plugindata.h (4) M src/pluginutils.c (16) M src/spawn.c (1) M src/stash.c (2) M src/ui_utils.c (29) M src/utils.c (15) M tagmanager/src/tm_workspace.c (4)
-- Patch Links --
https://github.com/geany/geany/pull/890.patch https://github.com/geany/geany/pull/890.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890
Sorry, but I'm not adding Ruby as a build dep. We already have Perl (required by Autotools), and a little Python. And docutils is Python, so already mostly an (optional) build-dep.
Why adding a 3rd (if not a gazillionth) scripting language? would this really be harder to do in Python or Perl? I get you probably are more comfortable in Ruby, but I don't think any other Geany dev is, and again, it adds a new scripting language -- and even a less common one at it.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175289837
Simple because I didn't find doxygen bindings for python or perl. I didn't chose it because I like it's better.
The dependency is optional, what's the problem?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175400691
@kugel- if you output the XML, you can just parse it directly in Python using xml.etree, or external lib like lxml. I had written [something similar](https://github.com/codebrainz/geany/blob/50f76094b06a2d7ae2a9ab7f1f594c052a4...) in XSLT to transform Doxygen's XML output to usable XML for generating some bindings. I bet one could re-write that XSLT into Python with about the same lines of code, so if that Ruby code doesn't do too much more (I didn't look at it as I don't know Ruby at all) then it should be reasonably simple/short to do it in Python without needing special interpreters and libraries and such.
It might also be possible to import classes from the [Breathe parser](https://github.com/michaeljones/breathe/tree/master/breathe/parser).
@b4n +1, we should be trying to reduce the number of scripting languages used, not adding more. Also, you forgot to mention PHP, we have a PHP script also (at least some core devs know PHP though).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175421628
The ruby code basically just loops through all enums, structs, typdedefs and functions, reading out the descriptions and decls. The bulk work is done inside the doxyparser lib (and nokogiri for pure xml parser) that finds all the elements in the bulk of the xml output files.
If I needed to write that in python it would probably end up 5k lines of code instead of 500.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175423973
BTW, There are already two ruby scripts in the tree, so I'm surprised this one is a problem given its optional.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175424878
Its not really optional since it won't be possible to make introspecting plugins without it.
On 27 January 2016 at 16:19, Thomas Martitz notifications@github.com wrote:
BTW, There are already two ruby scripts in the tree, so I'm surprised this one is a problem given its optional.
— Reply to this email directly or view it on GitHub https://github.com/geany/geany/pull/890#issuecomment-175424878.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175427733
There are already two ruby scripts in the tree, so I'm surprised this one is a problem given its optional.
Are you sure your not grepping into the CTags test dir? None of the ones we maintain in the `scripts` dir are in Ruby, they're all in language that multiple core developers know.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175427935
If I needed to write that in python it would probably end up 5k lines of code instead of 500.
Parsing XML is a piece of cake (in Python), especially if you use XPath like that XSLT I linked to.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175428878
My experience with python and xml has been a pita (yes I did parse xml with python in the past). Perhaps I didn't use the right tool/lib though (if they existed back then).
Still, we have two ruby scripts already and this one is optional* too so rejecting it just because it's ruby is unfair (I've invested a fair amount of time in it already).
*not optional when generating the tarball but we could just check the generated headers into git if that's the deal
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175429484
Am 27.01.2016 um 07:28 schrieb Matthew Brush:
There are already two ruby scripts in the tree, so I'm surprised this one is a problem given its optional.Are you sure your not grepping into the CTags test dir? None of the ones we maintain in the |scripts| dir are in Ruby, they're all in language that multiple core developers know.
Sorry my bad, they're python. My eyes lied to me. Perhaps I wanted to see ruby where there's none :-(
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175431942
rejecting it just because it's ruby is unfair.
I think rejecting adding code to the source tree based on it being written in a language none of the Geany developers know is not really unfair. I'd say it's even a reasonable minimum bar for determining what code gets added is that it's written in a language at least one core developer can review and fix.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175434324
I don't think we should check generated files into git, but putting them in the tarball so any tool is only a "build-from-git" requirement makes it less of a problem (so long as it is properly documented, probably in readme).
That said @codebrainz point that Ruby is less known to the Geany devs is relevant too.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175435701
As pointed out, it's only optional so long as you don't want GI. And as you want it, it's not really optional.
This said, I didn't see you were using an existing library to do some of the work. But unfortunately, this library isn't available in Debian apparently, so it'd mean we also either bundle it, or depend on the user fetching it somehow (without having searched for it, I myself have no clue how or where). Meh. Maybe I'm underestimating it, but like @codebrainz I wonder if it really can be that hard to parse some XML, which is supposed to be relatively easy to traverse.
But well. Yeah I understand you invested time in writing this, and it might not be nice to hear it's getting refused because it's written in a particular language. But without re-stating the arguments above, I don't think it's unfair, it's just not something I'm (we're) ready to commit to, nor find reasonable. Also, I doubt it'd throw all your efforts away. It's likely possible to translate most of the script to e.g. Python without having to start fro scratch, even if it will be some work.
Also, you forgot to mention PHP, we have a PHP script also (at least some core devs know PHP though).
AFAIK the only PHP script we have is to generate the PHP tags file, which is currently checked in. And well, I wouldn't have recommended using PHP, but I would also probably accept more easily a Ruby script if it was here only to generate some Ruby-specific stuff (e.g. I could understand that it maybe could access some internal AST or whatnot and that being interesting and very hard to reproduce in some other language). And yeah, I also happen to understand PHP quite well, so it's also one less barrier ^^
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175633167
As with python, ruby libraries can be installed with a ruby-specific package manager: gem install doxyparser. doxyparser in turn requires nokogiri, which can be installed the same way (but is also available as a debian package). So whether or not being available as a debian is not an issue IMO.
Traversing XML is one thing, but dealing with the doxygen-specifics is a lot different, e.g. how all the commands (@param, @see, @a etc.) end up in the XML and how descriptions can contain basically arbitrary markup.
Anyway, @codebrainz tought me a bit already about python-lxml and XPath usage so maybe I can find a way.
It's still depressing since you guys knew very early that I use Ruby and there was never any sign of "not going to accept this". This is just frustrating very much.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175636371
@kugel- Yeah, understand its frustrating, sorry for that, but none of us is a machine (comments about b4nbot notwithstanding :) and so its not surprising that everyone missed the point of your previous note you were using Ruby.
And needing to use a language specific package installer rather than the distro package manager is another negative. For example the Python docutils tools for the manual would be frowned on if they were only available from `pip install` or other Python specific thing, not a distro package. Which means probably you should only be using the Python standard library or packaged libraries (noted ahead of time :).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-175956303
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
should rather be 3 lines in the rule, I don't see we need shell here and it's less clear
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52635611
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
why introducing this new variable? AFAIK it's documented Make behavior to *add* dependencies when specifying a rule with only dependencies and no rules, so it would do the same as you do here… or am I missing some Automake weirdness?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52636026
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
BTW, will Doxygen reuse some of its findings for the second run? I know you'll hate me because we wanted separate doxyfiles not to alter the existing doc, but as Docygen is relatively slow, if it could reuse its database or something it'd be super nice :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52637124
+Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
+ALL_TARGETS += geany-gtkdoc.h geany-scintilla-gtkdoc.h
+geany_includedir = $(includedir)/geany/gtkdoc
I'd prefer the variable to be names e.g. `geany_gtkdocincludedir` or something, not to create confusing with the normal includedir in src
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52637603
@@ -70,3 +70,16 @@ AC_DEFUN([GEANY_CHECK_DOCUTILS_PDF], AM_CONDITIONAL([WITH_RST2PDF], [test "x$geany_enable_pdf_docs" != "xno"]) GEANY_STATUS_ADD([Build PDF documentation], [$geany_enable_pdf_docs]) ])
+dnl +dnl GEANY_CHECK_PYTHON +dnl For gtkdoc header generation +dnl +AC_DEFUN([GEANY_CHECK_PYTHON], +[
- AM_PATH_PYTHON([2.7], [], [])
does it work being optional if the 3rd argument is totally empty like this? otherwise, just use the shell no-op `:` as the content of the third
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52638581
+(opts,args) = parser.parse_args()
+xml_dir = args[0] +if (opts.outfile):
- outfile = open(opts.outfile, "w+")
+else:
- outfile=sys.stdout
+if (opts.scioutfile):
- scioutfile = open(opts.scioutfile, "w+")
+else:
- scioutfile = outfile
+if (outfile is None):
- sys.stderr.write("no output file\n")
- exit(1)
should also check `scioutfile`
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52639109
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
Am 11. Februar 2016 18:38:18 MEZ, schrieb Colomban Wendling notifications@github.com:
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo ""
$@
BTW, will Doxygen reuse some of its findings for the second run? I know you'll hate me because we wanted separate doxyfiles not to alter the existing doc, but as Docygen is relatively slow, if it could reuse its database or something it'd be super nice :)
Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52637124
No, you can't have both. This was one of the reasons for me preferring a single doxyfile
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52639416
for n in xml.getchildren():if n.tag == "emphasis":s += self.at.cb("a", self.__process_element(n))if n.tag == "computeroutput":s += self.at.cb("c", self.__process_element(n))if n.tag == "itemizedlist":s += "\n" + self.__process_element(n)if n.tag == "listitem":s += " - " + self.__process_element(n)if n.tag == "para":s += self.__process_element(n) + "\n"if n.tag == "ref":s += n.text if n.text else ""if n.tag == "simplesect":ss = self.at.cb(n.get("kind"), self.__process_element(n))s += ss if ss + "\n" else ""
looks wrong, as it's a convoluted way to write `ss` (as adding the `"\n"` in the test makes it evaluate to true). Shouldn't the `\n` be added on the other value rather than the test?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52641182
+outfile.write("typedef struct _ScintillaObject ScintillaObject;\n") +outfile.write("typedef struct TMSourceFile TMSourceFile;\n") +outfile.write("typedef struct TMWorkspace TMWorkspace;\n")
+# write typedefs first, they are possibly undocumented but still required (even +# if they are documented, they must be written out without gtkdoc) +for e in typedefs:
- outfile.write(e.definition)
- outfile.write("\n\n")
+for e in filter(lambda x: x.is_documented(), other):
- outfile.write("\n\n")
- outfile.write(e.to_gtkdoc())
- outfile.write(e.definition)
- outfile.write("\n\n")
- if (e.name.startswith("sci_")):
won't this break sciwrappers?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52641914
@@ -113,6 +113,9 @@ GeanyFiletypeID;
#define filetype_id GeanyFiletypeID /* compat define - should be removed in the future */
+/** Filetype categories
- These are used to provide submenus for each category in the GUI */
should probably be only GIR, shouldn't it?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52642385
@@ -1955,11 +1955,12 @@ static gboolean str_in_array(const gchar **haystack, const gchar *needle)
- The argument list must be @c NULL-terminated.
spurious
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52642903
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"
should probably rather be named something like `gitonly` as it's not very clear now when it appears in the docs
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52646652
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly"
should probably be `girskip` to be clearer
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52646757
I believe there are a lot of transfer annotations that aren't needed, and in most case natural to the reader. In such case, I guess I'd rather leave them out.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183005770
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
this doesn't properly check for failure. Something like this is needed: ```diff diff --git a/doc/Makefile.am b/doc/Makefile.am index 3396063..1f97e4c 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -123,7 +123,8 @@ geany-scintilla-gtkdoc.h: geany-gtkdoc.h
geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \ - -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h + -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h \ + || { $(RM) geany-gtkdoc.h geany-scintilla-gtkdoc.h && exit 1; }
ALL_TARGETS += geany-gtkdoc.h geany-scintilla-gtkdoc.h
```
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52648965
Hum, for some reason the setup breaks if you have some old stuff in your doc/ directory… I had something called `interface.{dox,dox.in,xml,xslt}` and it broke the XML generation somehow. This should probably not be the case as it's kind of fragile, but well, with those gone the build worked.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183013025
Just thinking: all those struct members are visible as public to GIR, shouldn't the private one have a GtkDoc `/*<private>*/` comment above them and public ones a `/*<public>*/` comment? Or alternatively if those aren't enough for GIT, maybe the private ones should get a `private__` prefix or alike, and a documentation blurb saying "private"? I'm just saying this because GIR is introspectable by nature, so there's virtually nothing suggesting the user it's not public.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183017055
Apart all that, looks like it does as advertized. Yet, what's this about this scintilla header? OK, I get it's nice if those are method on ScintillaObject, but the symbols doesn't exist so…?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183018989
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly"
not used (?)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52651949
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly"
any reason why not using the actual GtkDoc name?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52652062
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly" +ALIASES += "cb=\a \xmlonly <simplesect kind="geany:cb"></simplesect>\endxmlonly" +ALIASES += "cbdata=\a \xmlonly <simplesect kind="geany:cbdata"></simplesect>\endxmlonly" +ALIASES += "cbfree=\a \xmlonly <simplesect kind="geany:cbfree"></simplesect>\endxmlonly"
mapping those last 2 (or even 3) to the gtkdoc name directly here would make the parsing script simpler by not doing another translation. Like ``` ALIASES += "cb=\a \xmlonly <simplesect kind="geany:scope">notified</simplesect>\endxmlonly" ALIASES += "cbdata=\a \xmlonly <simplesect kind="geany:closure"></simplesect>\endxmlonly" ALIASES += "cbfree=\a \xmlonly <simplesect kind="geany:destroy"></simplesect>\endxmlonly"
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52652858
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly" +ALIASES += "cb=\a \xmlonly <simplesect kind="geany:cb"></simplesect>\endxmlonly" +ALIASES += "cbdata=\a \xmlonly <simplesect kind="geany:cbdata"></simplesect>\endxmlonly" +ALIASES += "cbfree=\a \xmlonly <simplesect kind="geany:cbfree"></simplesect>\endxmlonly"
BTW, maybe making those have the ns `gtkdoc` instead of `geany` would be a little clearer? Not important though
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52653059
typedefs.append(e)- for n0 in f.xpath(".//*/memberdef[@kind='enum' and @prot='public']"):
e = DoxyEnum.from_memberdef(n0)other.append(e)+for n0 in root.xpath(".//compounddef[@kind='struct' and @prot='public']"):
- e = DoxyStruct.from_compounddef(n0)
- other.append(e)
+for f in c_files:
- for n0 in f.xpath(".//*/memberdef[@kind='function' and @prot='public']"):
e = DoxyFunction.from_memberdef(n0)other.append(e)+outfile.write("#include <glib.h>\n")
should probably have a comment stating that it is generated (same for scioutfile)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52653853
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
Seems unecessary to me? Is this just to make sure *.h dont exist if the script failed?
Apart from that, the script can only fail early. It can't fail between the two output files (execpt file system errors).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671135
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"
ok
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671226
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly"
right. will drop it for now
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671324
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly"
girskip sounds better and is consistent with gironly.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671387
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly"
null is shorter. but I'll change to nullable
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671428
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly" +ALIASES += "cb=\a \xmlonly <simplesect kind="geany:cb"></simplesect>\endxmlonly" +ALIASES += "cbdata=\a \xmlonly <simplesect kind="geany:cbdata"></simplesect>\endxmlonly" +ALIASES += "cbfree=\a \xmlonly <simplesect kind="geany:cbfree"></simplesect>\endxmlonly"
i prefer geany. it doesnt really matter though, it's just to transport stuff to the script. the xml is not used outside the header generation.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52671898
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
should rather be 3 lines in the rule, I don't see we need shell here and it's less clear
It was a single line before (including shell usage). I just extended it. Can change to three lines if you prefer.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672100
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
I prefer this style generally because it's too easy to accidentally add a recipe to one of the targets and get unwanted results. I also always find multiple occurrences of the same targets confusing most of the time.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672514
@@ -70,3 +70,16 @@ AC_DEFUN([GEANY_CHECK_DOCUTILS_PDF], AM_CONDITIONAL([WITH_RST2PDF], [test "x$geany_enable_pdf_docs" != "xno"]) GEANY_STATUS_ADD([Build PDF documentation], [$geany_enable_pdf_docs]) ])
+dnl +dnl GEANY_CHECK_PYTHON +dnl For gtkdoc header generation +dnl +AC_DEFUN([GEANY_CHECK_PYTHON], +[
- AM_PATH_PYTHON([2.7], [], [])
Yes it does. It's important to not use the short form (AM_PATH_PYTHON([2.7])) to avoid the default actions being used.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672649
+(opts,args) = parser.parse_args()
+xml_dir = args[0] +if (opts.outfile):
- outfile = open(opts.outfile, "w+")
+else:
- outfile=sys.stdout
+if (opts.scioutfile):
- scioutfile = open(opts.scioutfile, "w+")
+else:
- scioutfile = outfile
+if (outfile is None):
- sys.stderr.write("no output file\n")
- exit(1)
ok
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672693
for n in xml.getchildren():if n.tag == "emphasis":s += self.at.cb("a", self.__process_element(n))if n.tag == "computeroutput":s += self.at.cb("c", self.__process_element(n))if n.tag == "itemizedlist":s += "\n" + self.__process_element(n)if n.tag == "listitem":s += " - " + self.__process_element(n)if n.tag == "para":s += self.__process_element(n) + "\n"if n.tag == "ref":s += n.text if n.text else ""if n.tag == "simplesect":ss = self.at.cb(n.get("kind"), self.__process_element(n))s += ss if ss + "\n" else ""
you are completely right.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672784
typedefs.append(e)- for n0 in f.xpath(".//*/memberdef[@kind='enum' and @prot='public']"):
e = DoxyEnum.from_memberdef(n0)other.append(e)+for n0 in root.xpath(".//compounddef[@kind='struct' and @prot='public']"):
- e = DoxyStruct.from_compounddef(n0)
- other.append(e)
+for f in c_files:
- for n0 in f.xpath(".//*/memberdef[@kind='function' and @prot='public']"):
e = DoxyFunction.from_memberdef(n0)other.append(e)+outfile.write("#include <glib.h>\n")
ok
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52672803
+outfile.write("typedef struct _ScintillaObject ScintillaObject;\n") +outfile.write("typedef struct TMSourceFile TMSourceFile;\n") +outfile.write("typedef struct TMWorkspace TMWorkspace;\n")
+# write typedefs first, they are possibly undocumented but still required (even +# if they are documented, they must be written out without gtkdoc) +for e in typedefs:
- outfile.write(e.definition)
- outfile.write("\n\n")
+for e in filter(lambda x: x.is_documented(), other):
- outfile.write("\n\n")
- outfile.write(e.to_gtkdoc())
- outfile.write(e.definition)
- outfile.write("\n\n")
- if (e.name.startswith("sci_")):
no, why should it? sci_* is still part of geany-gtkdoc.h
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52673003
@@ -113,6 +113,9 @@ GeanyFiletypeID;
#define filetype_id GeanyFiletypeID /* compat define - should be removed in the future */
+/** Filetype categories
- These are used to provide submenus for each category in the GUI */
Yes (this was made before the other decision I still don't agree with).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52673217
@@ -1955,11 +1955,12 @@ static gboolean str_in_array(const gchar **haystack, const gchar *needle)
- The argument list must be @c NULL-terminated.
ok
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52673261
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
Well, my files that broke the generation didn't break early enough for the header not to have been created, so the next run though the file was up to date. So the target should definitely be remove on failure
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52673744
@@ -247,7 +247,18 @@ ALIASES = "signal=- @ref " \ "endsignalproto=@endcode " \ "signaldesc=" \ "signals=@b Signals: " \
"endsignals= "
"endsignals= " \"addtogir=@internal"+ALIASES += "transfer{1}=\a \xmlonly <simplesect kind="geany:transfer">\1</simplesect>\endxmlonly \htmlonly (transfer: \1) \endhtmlonly" +ALIASES += "elementtype{1}=\a \xmlonly <simplesect kind="geany:element-type">\1</simplesect>\endxmlonly \htmlonly (element-type: \1) \endhtmlonly" +ALIASES += "scope{1}=\a \xmlonly <simplesect kind="geany:scope">\1</simplesect>\endxmlonly \htmlonly (scope: \1) \endhtmlonly" +ALIASES += "skip=\a \xmlonly <simplesect kind="geany:skip"></simplesect>\endxmlonly" +ALIASES += "null=\a \xmlonly <simplesect kind="geany:nullable"></simplesect>\endxmlonly"
I just find "nullable" more explicit and consistent with gtkdoc
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52674256
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
I do. it was a bad idea before, and gets worse by adding more logic to the line
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52674471
Am 11.02.2016 um 20:19 schrieb Colomban Wendling:
Apart all that, looks like it does as advertized. Yet, what's this about this scintilla header? OK, I get it's nice if those are method on ScintillaObject, but the symbols doesn't exist so…?
Actually, it seems I messed up rebasing. The purpose geany-scintilla-gtkdoc.h is to hold ScintillaObject-related stuff. As you guessed rightly, it includes actual methods, which are our sci_ functions aliased to scintilla_object_ (only with this GIR recognizes this as methods).
Now, I didn't include the code/script to generate the scintilla_object_* methods out of sci_* in this PR (it's another small python script, and uses __attribute__((alias)) as I've failed to find a better solution yet). Therefore the scintilla_object_* functions are missing too.
Still, since I eventually plan to fully implement the scintilla_object_* stuff and use a separate header for that, the header should already be created now and include the ScintillaObject and ScintillaObjectClass definitions. I'll update the PR accordingly.
(A separate header in order to generate a separate GeanyScintilla-1.0.gir and GeanyScintilla-1.0.typelib. So that you import GeanyScintilla without importing Geany)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183079815
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
I don't understand. The output files are created last in the script and that doesn't depend on the xml anymore (it writes out whatever it parsed, after parsing is completed).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52676004
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
Am 11.02.2016 um 23:00 schrieb Colomban Wendling:
In doc/Makefile.am https://github.com/geany/geany/pull/890#discussion_r52674471:
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
I do. it was a bad idea before, and gets worse by adding more logic to the line
Fine. I'll use $(AM_V_at) to supress output for all but the last line.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52676198
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
oh, echoing. Well, if it's important then maybe just split the lines with a \ so at least it's each instruction on its own line
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52678227
-all-local: Doxyfile.stamp +Doxyfile.stamp: Doxyfile Doxyfile-gi $(doxygen_sources)
- $(AM_V_GEN)$(DOXYGEN) Doxyfile-gi && $(DOXYGEN) Doxyfile && echo "" > $@
+ALL_TARGETS = Doxyfile.stamp
+if WITH_PYTHON
+geany-scintilla-gtkdoc.h: geany-gtkdoc.h
+geany-gtkdoc.h: Doxyfile.stamp $(top_srcdir)/scripts/gen-api-gtkdoc.py
- $(AM_V_GEN)$(top_srcdir)/scripts/gen-api-gtkdoc.py xml -d $(builddir) \
- -o geany-gtkdoc.h --sci-output geany-scintilla-gtkdoc.h
well, I guess open() creates the file or something, because when the script stopped at line 64 I definitely had to clean those up manually
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r52678368
Now, I didn't include the code/script to generate the scintilla_object_* methods out of sci_* in this PR (it's another small python script, and uses __attribute__((alias)) as I've failed to find a better solution yet). Therefore the scintilla_object_* functions are missing too.
It'd be nice it it was possible to user `rename-to` annotation, but apparently that only works if the target name does exist…
[…] I eventually plan to fully implement the scintilla_object_* stuff […]
what do you mean?
(A separate header in order to generate a separate GeanyScintilla-1.0.gir and GeanyScintilla-1.0.typelib. So that you import GeanyScintilla without importing Geany)
Hum… why do you want this separation?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183086682
On 2016-02-11 11:06 AM, Colomban Wendling wrote:
Hum, for some reason the setup breaks if you have some old stuff in your doc/ directory… I had something called `interface.{dox,dox.in,xml,xslt}` and it broke the XML generation somehow. This should probably not be the case as it's kind of fragile, but well, with those gone the build worked.
Probably from my previously linked codegen thing I had worked on before:
https://github.com/codebrainz/geany/blob/50f76094b06a2d7ae2a9ab7f1f594c052a4...
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183145790
In case not everyone gets notifications from the PR branch on @kugel- fork, I added a PR which I think contains small but good changes. Someone better at Autotools should review the Autoconf improvement there.
https://github.com/kugel-/geany/pull/5
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183146796
Am 11.02.2016 um 23:33 schrieb Colomban Wendling:
Now, I didn't include the code/script to generate the scintilla_object_* methods out of sci_* in this PR (it's another small python script, and uses *attribute*((alias)) as I've failed to find a better solution yet). Therefore the scintilla_object_* functions are missing too.It'd be nice it it was possible to user |rename-to| annotation, but apparently that only works if the target name does exist…
Yes, rename-to isn't suitable. I've tried...
[…] I eventually plan to fully implement the scintilla_object_* stuff […]what do you mean?
I plan to find an acceptable solution to provide sci_ as methods for ScintillaObject (as far as gir is concerned), and implement it so that Geany can ship these methods.
(A separate header in order to generate a separate GeanyScintilla-1.0.gir and GeanyScintilla-1.0.typelib. So that you import GeanyScintilla without importing Geany)Hum… why do you want this separation?
IMO it makes sense to split out GeanyScintilla from a design pov. scintilla adds so many symbols which can be saved if you don't need scintilla stuff (think of "from gi.repository import Geany" not pulling lots of scintilla symbols).
Also it allows scintilla to be used without geany parts. I expect that could become common for peasy sub-plugins, which want to use the GeanyScintilla module (assuming they mess with edited text), but not Geany because Peasy provides the preffered API wrappers.
At the end of the day it doesn't make a huge difference, but it's my personal preference to do the split.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-183978981
Am 11.02.2016 um 19:45 schrieb Colomban Wendling:
I believe there are a lot of transfer annotations that aren't needed, and in most case natural to the reader. In such case, I guess I'd rather leave them out.
@b4n Which ones? I only added them where needed, that is basically all non-const returns of struct pointers (otherwise g-ir-scanner throws warnings like this:
Geany: editor_create_widget: return value: Invalid non-constant
return of bare structure or union; register as boxed type or (skip)
The only superflous annotation was for stash_group_new(). I dropped that. However, IMO the transfer annotations are very valuable for the html/pdf docs to and probably shouldn't be dropped either way.
@codebrainz The updated patch set incorporates your changes.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-184712949
+class AtDoc(object):
- def __init__(self):
self.retval = Noneself.since = ""self.annot = []- def cb(self, type, str):
if (type == "param"):words = str.split(" ", 2);self.annot = []elif (type == "return"):self.annot = []elif (type == "since"):self.since = str.rstrip()elif (type == "geany:skip"):
this is handled right below again
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53341661
self.since = ""self.annot = []- def cb(self, type, str):
if (type == "param"):words = str.split(" ", 2);self.annot = []elif (type == "return"):self.annot = []elif (type == "since"):self.since = str.rstrip()elif (type == "geany:skip"):self.annot.append("skip")elif (type == "geany:nullable") or (type == "geany:skip"):self.annot.append(type.split(":")[1])elif (type == "geany:cb"):
this isn't needed anymore
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53341788
words = str.split(" ", 2);self.annot = []elif (type == "return"):self.annot = []elif (type == "since"):self.since = str.rstrip()elif (type == "geany:skip"):self.annot.append("skip")elif (type == "geany:nullable") or (type == "geany:skip"):self.annot.append(type.split(":")[1])elif (type == "geany:cb"):self.annot.append("scope notified")elif (type == "geany:cbdata"):self.annot.append("closure")elif (type == "geany:cbfree"):self.annot.append("destroy")
these two are the old version, now should be `geany:closure` and `geany:destroy`. So, can be merged with the generic `geany:` prefix stripping handling.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53342021
@@ -1651,7 +1651,7 @@ const gchar *utils_get_default_dir_utf8(void)
- @param argv The child's argument vector.
- @param env The child's environment, or @c NULL to inherit parent's.
- @param flags Ignored.
- @param child_setup Ignored.
- @param child_setup @girskip Ignored.
- @param user_data Ignored.
shouldn't this be also ignored? or is it automatically because it's the data of the previous func, even if it's explicitly ignored?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53343287
- @param exclude_vars @c NULL-terminated array of variable names to exclude.
- @param first_varname Name of the first variable to copy into the new array.
- @param ... Key-value pairs of variable names and values, @c NULL-terminated.
- @return The new environment array. Use @c g_strfreev() to free it.
- @return @transfer{full} The new environment array. Use @c g_strfreev() to free it.
`@type{GStrv}` :]
Doesn't returning non-const pointers default to `transfer: full`?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53343521
I believe there are a lot of transfer annotations that aren't needed, and in most case natural to the reader. In such case, I guess I'd rather leave them out.
@b4n Which ones? I only added them where needed, that is basically all non-const returns of struct pointers (otherwise g-ir-scanner throws warnings like this:
Geany: editor_create_widget: return value: Invalid non-constant
return of bare structure or union; register as boxed type or (skip)
Hum, okay. I thought if you returned e.g. a `GeanyDocument` pointer it would have had `transfer: none` by default if not a known type or a `_new()` function, but if it's needed OK.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-185814167
Added some things to https://github.com/b4n/geany/commits/b4n/kugel-/gtkdoc-hdr:
* e9e2ddf581f1a510e5fdc63b78da444094912908 Add an option to control GtkDoc header generation. This is useful both to force-enable and force-disable the thing, and more robust that letting the deps do all the work. * 240b5903be6be32b4f5050878bc0263ecee84b25 Fix distcheck. * 91a8bb6c6aa155085be370e9301cca324cea8914 Don't generate GtkDoc header stuff if GtkDoc header will not be built. E.g. move *all* the gtkdoc header-specifics to the appropriate conditional. Also helps encapsulation. * 66c3323afbe88c7519dd7084825d4330b201dd01 Force enable GtkDoc header on distcheck. Probably what (you) want.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-185969114
proc = DoxygenProcess()self.brief = proc.process_element(xml)self.extra += proc.get_extra()- def add_detail(self, xml):
proc = DoxygenProcess()self.detail = proc.process_element(xml)self.extra += proc.get_extra()self.since = proc.get_since()- def add_member(self, xml):
name = xml.find("name").textproc = DoxygenProcess()brief = proc.process_element(xml.find("briefdescription"))# optional doxygen command output appears within <detaileddescription />proc.process_element(xml.find("detaileddescription"))
apparently processing the detailed description like this erases any possible annotation from the brief description, which is not handy for members
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53399933
Added more stuff to https://github.com/b4n/geany/commits/b4n/kugel-/gtkdoc-hdr:
* 91daa9f9db0b83f069388c954bf985a4436f42fa Fix handling of some GtkDoc annotations (as per my comments here) * 891b07199194a0ed2a3335b869899d297688b07b Add support for the "type" GtkDoc annotation (as suggested in my comments here) * 398f238f9b886d74b09bd147ab5481be8b560c66 Use the "type" annotation for GStrv (as suggested, not necessarily what you prefer)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-185978516
@@ -70,3 +70,16 @@ AC_DEFUN([GEANY_CHECK_DOCUTILS_PDF], AM_CONDITIONAL([WITH_RST2PDF], [test "x$geany_enable_pdf_docs" != "xno"]) GEANY_STATUS_ADD([Build PDF documentation], [$geany_enable_pdf_docs]) ])
+dnl +dnl GEANY_CHECK_PYTHON +dnl For gtkdoc header generation +dnl +AC_DEFUN([GEANY_CHECK_PYTHON], +[
- AM_PATH_PYTHON([2.7], [], [])
BTW, do we really need Python as recent as 2.7?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53464663
@@ -70,3 +70,16 @@ AC_DEFUN([GEANY_CHECK_DOCUTILS_PDF], AM_CONDITIONAL([WITH_RST2PDF], [test "x$geany_enable_pdf_docs" != "xno"]) GEANY_STATUS_ADD([Build PDF documentation], [$geany_enable_pdf_docs]) ])
+dnl +dnl GEANY_CHECK_PYTHON +dnl For gtkdoc header generation +dnl +AC_DEFUN([GEANY_CHECK_PYTHON], +[
- AM_PATH_PYTHON([2.7], [], [])
You must be kidding me right? Python 2.7 is over 5 years old. python 3 is even older (nearly 8 years). "Recent" is not quite right. Even depending on py3 would be perfectly reasonable but I still made the effort to make it 2.7 compatible.
Anyway, I dont have something older than 2.7 so I can't actually tell. Might work with older versions.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53467812
I added some of @b4n commits (all except the last two ones, those about "@type"). Hope this can be merged.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186248020
proc = DoxygenProcess()self.brief = proc.process_element(xml)self.extra += proc.get_extra()- def add_detail(self, xml):
proc = DoxygenProcess()self.detail = proc.process_element(xml)self.extra += proc.get_extra()self.since = proc.get_since()- def add_member(self, xml):
name = xml.find("name").textproc = DoxygenProcess()brief = proc.process_element(xml.find("briefdescription"))# optional doxygen command output appears within <detaileddescription />proc.process_element(xml.find("detaileddescription"))
Members are usually commented after the semicolon. In this case there is only a briefdescription. They can also be commented like functions in which case annotations are expected in the detaildescription (I'm not sure we have any of these currently?).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53470752
proc = DoxygenProcess()self.brief = proc.process_element(xml)self.extra += proc.get_extra()- def add_detail(self, xml):
proc = DoxygenProcess()self.detail = proc.process_element(xml)self.extra += proc.get_extra()self.since = proc.get_since()- def add_member(self, xml):
name = xml.find("name").textproc = DoxygenProcess()brief = proc.process_element(xml.find("briefdescription"))# optional doxygen command output appears within <detaileddescription />proc.process_element(xml.find("detaileddescription"))
I forgot to add: in the first case, the detaildescription part has only annotations (all text is in the briefdescription). This is because @command appear after the dot of the brief sentence.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53470941
proc = DoxygenProcess()self.brief = proc.process_element(xml)self.extra += proc.get_extra()- def add_detail(self, xml):
proc = DoxygenProcess()self.detail = proc.process_element(xml)self.extra += proc.get_extra()self.since = proc.get_since()- def add_member(self, xml):
name = xml.find("name").textproc = DoxygenProcess()brief = proc.process_element(xml.find("briefdescription"))# optional doxygen command output appears within <detaileddescription />proc.process_element(xml.find("detaileddescription"))
Mmm'kay, just mentioned that because I tried to add the @type before the brief and it was missing from the output.
But it may be bad some annotations might just disappear, which would create weird bugs in the generated output.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53482613
Hope this can be merged.
Not before something is done for GStrv, currently the build fails with any GLib older than 2.46.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186288780
BTW, the GtkDoc output contains a lot of bare `@`, I'd say one for each command in the input. Couldn't that be fixed, and doesn't it confuse GI?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186289172
Also, what about the `/*<private>*/` idea, would it be feasible without too much pain?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186289356
@@ -70,3 +70,16 @@ AC_DEFUN([GEANY_CHECK_DOCUTILS_PDF], AM_CONDITIONAL([WITH_RST2PDF], [test "x$geany_enable_pdf_docs" != "xno"]) GEANY_STATUS_ADD([Build PDF documentation], [$geany_enable_pdf_docs]) ])
+dnl +dnl GEANY_CHECK_PYTHON +dnl For gtkdoc header generation +dnl +AC_DEFUN([GEANY_CHECK_PYTHON], +[
- AM_PATH_PYTHON([2.7], [], [])
You must be kidding me right? Python 2.7 is over 5 years old. python 3 is even older (nearly 8 years). "Recent" is not quite right. Even depending on py3 would be perfectly reasonable but I still made the effort to make it 2.7 compatible. Anyway, I dont have something older than 2.7 so I can't actually tell. Might work with older versions.
I'm not kidding or suggesting 2.7 is bad, I was mostly curious whether there was an actual reason for requiring 2.7, or if it was just that you didn't knew whether or not it worked on older versions.
I don't mind, at worse it disables the GtkDoc header, and us developers have it anyway I guess.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r53483227
Also, what about the `/*<private>*/` idea, would it be feasible without too much pain?
I did it naively in 7734718670ef1748aef3224a4cb9e5357c4617ba.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186303222
@b4n I fixed the build and also added your commit. The idea is nice, well done :-) Although now some structs are all-private (e..g GeanyEditorPrefs). This doesn't look intended but not sure what to do about it (doesn't seem to be a problem for my peasy plugin yet).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186690878
@b4n I fixed the build
Good :)
Although now some structs are all-private (e..g GeanyEditorPrefs). This doesn't look intended but not sure what to do about it (doesn't seem to be a problem for my peasy plugin yet).
Indeed, odd. That shows issues in the apidocs, yay :) At least some members are used by some plugins, so we could consider adding them. Good catch, opened #910 about it not to foget. I guess we should review the settings and decide what to add and what not.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-186693572
@b4n perhaps your last commit should be post-poned, e.g. GeanyEditorPrefs (the struct) is exported via GeanyData but no member is accessible. This points to an accident. (to me it shows once more that "undocumented members" != "private members", or that this definition is recently come up with).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-187201465
@kugel-, to quote the [plugin documentation](https://github.com/geany/geany/commit/5a6ee46699a92752de4c7c524431b4d2561e4a...):
Do not use anything not documented here, it may change.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-187234046
@b4n perhaps your last commit should be post-poned, e.g. GeanyEditorPrefs (the struct) is exported via GeanyData but no member is accessible. This points to an accident. […]
I'd rather have more things private and expose them later on rather than breaking GI API because we exposed too much.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-188396706
Okay. I'm just afraid I cannot use much of the api for a full cycle because of missing comments (bytes accident). Anyway, just merge it as-is if you like
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-188398273
Anyway, just merge it as-is if you like
I don't, there are still open points :)
* https://github.com/geany/geany/pull/890/files#r52642385 * https://github.com/kugel-/geany/commit/f2ebd288b378cbd2b5e9a2318e19b5a5ed410... * https://github.com/geany/geany/pull/890#discussion_r53343287 * https://github.com/geany/geany/pull/890#issuecomment-186289172 * https://github.com/geany/geany/pull/890#issuecomment-183013025 * https://github.com/geany/geany/pull/890#discussion_r52642903 (not important)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-188503754
Hum, for some reason the setup breaks if you have some old stuff in your doc/ directory… I had something called interface.{dox,dox.in,xml,xslt} and it broke the XML generation somehow. This should probably not be the case as it's kind of fragile, but well, with those gone the build worked.
Besides that these files are never created (I'm assuming @codebrainz is right and it's a left-over from one of his stuff), the python script only opens ever opens the xml subdirectory, and in their it uses the doxygen output (combine.xslt) to learn which other xml it has to parse. Are you sure this still happens? What's the exact error?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-188627188
Am 25.02.2016 um 00:07 schrieb Colomban Wendling:
Anyway, just merge it as-is if you likeI don't, there are still open points :)
Fixed some of them. Hopefully mergeable now.
The stray @ are unproblematic as far as .gir generation is concerned. They are caused by the extra \a command in the front of each of the gir-specific commands (see ALIASES in Doxyfile.in). I can't remember the details actually but I needed them to generate the gir (lots of error messages come up when the \a are removed).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-188793577
Besides that these files are never created (I'm assuming @codebrainz is right and it's a left-over from one of his stuff), the python script only opens ever opens the xml subdirectory, and in their it uses the doxygen output (combine.xslt) to learn which other xml it has to parse. Are you sure this still happens? What's the exact error?
``` GEN geany-gtkdoc.h Traceback (most recent call last): File "../../scripts/gen-api-gtkdoc.py", line 415, in <module> sys.exit(main(sys.argv)) File "../../scripts/gen-api-gtkdoc.py", line 352, in main root = transform(doc) File "src/lxml/xslt.pxi", line 579, in lxml.etree.XSLT.__call__ (src/lxml/lxml.etree.c:187342) File "src/lxml/lxml.etree.pyx", line 322, in lxml.etree._ExceptionContext._raise_if_stored (src/lxml/lxml.etree.c:11069) lxml.etree.XSLTApplyError: Cannot resolve URI xml/interface_8dox.xml ``` Fetching `interface.dox` and `interface.xslt` from @codebrainz's repo https://github.com/codebrainz/geany/blob/50f76094b06a2d7ae2a9ab7f1f594c052a4...
But forget it, it's not your PR's fault but the `$(srcdir)/*.dox` entry in `doxygen_sources` that trigger inclusion of `interface.dox`, which then refers non-generated things apparently.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-189726302
https://github.com/b4n/geany/commits/b4n/kugel-/gtkdoc-hdr_2 fixes or adds some extra nice stuff
* avoids extra `@`s in the GtkDoc output (cosmetic, but visible in the GIR docs section) * use of `%TRUE`, `%FALSE` and `%NULL` in the GtkDoc output to mark those as constants as appropriate (cosmetic, but visible in the GIR docs section) * Improve HTML version of the annotations * Mark output parameter as such (parameters in which the function writes a value rather than reads it) * Add a bunch of `@nullable`, which might be very useful to GIR in some places, and could be nice in HTML everywhere (but not yet rendered)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-189761358
Happy with your commits, thank you!
(IMO this PR could use some squashing but meh)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-189963990
(IMO this PR could use some squashing but meh)
Yeah but it looks very annoying to do properly, so I think it'll be fine like it is.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-189969643
@kugel- I added a license header to the generation script (last commit on https://github.com/b4n/geany/commits/b4n/kugel-/gtkdoc-hdr_2). If you're fine with it I'll merge this, looks good (enough).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-189969915
- [x] I'm fine
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-190076225
- Removes C++ name qualifications from some definitions.
- For example:
- 'bool flag'
- 'bool flag'
- 'void(* project_open) (GKeyFile *keyfile)'
- """
- return CXX_NAMESPACE_RE.sub(r"", s)
+class AtAt(object):
unused (?)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r54579544
type = type.split(":")[1]self.annot.append("%s %s" % (type, str))elif (type == "see"):return "See " + strelif type in ("a", "c") and str in ("NULL", "TRUE", "FALSE"):# FIXME: some of Geany does @a NULL instead of @c NULLreturn "%" + strelif (type == "a"):return "@" + strelse:return strreturn ""+class At(object):
Unused (?)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890/files#r54579563
@kugel- Can I remove the two unused classes `At` and `AtAt`, or am I missing something?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-190762845
Sorry, more stuff: https://github.com/b4n/geany/commits/b4n/kugel-/gtkdoc-hdr_2
* Remove the unused classes * Make Doxygen itself ignore `G_BEGIN`_DECLAS` and alike instead of manually stripping them. This also improves the HTML output, while making Doxygen C parsing more robust (as it won't have to deal with those preproc symbols)
Checked both generated XML and header for changes, look just fine as you might expect.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#issuecomment-190775883
Merged #890.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/890#event-573561357
github-comments@lists.geany.org