[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
Merged #3049 into master. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#event-10674470211 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
Great, so I've just re-pushed the squashed version in case someone wants to have a look at it. I'll merge it tomorrow (well, actually later today) but feel free to merge it earlier if you want. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1765411983 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr approved this pull request. LGBI, docs ok, I won't be home to be able to test anything until after release unfortunately -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1681070153 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> Apart the small doc changes, LGTM. Committed now. Alright, I'll wait for @elextr if he has some more comments to the documentation, then squash the commits a bit and merge the result afterwards. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1765385506 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@b4n approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1681065041 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. 69ad323270615947ae6249c9eab2396c7301cf43 Add a remark about tag file naming when 'l' field is used -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/bc49cd2750dacf0bb5f82b7f30d328df79f23e9e..69ad323270615947ae6249c9eab2396c7301cf43 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@kugel- approved this pull request. LGBI but I only understand a fraction of it anyway -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1680935257 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@b4n commented on this pull request.
> {
- if ((NULL == fgets(buf, BUFSIZ, fp)) || ('\0' == *buf))
- return FALSE;
- }
- while (strncmp(buf, "!_TAG_", 6) == 0); /* skip !_TAG_ lines */
+ TMTagType type;
+ TMTag *tag;
+ guint i, idx;
yeah, the future is knocking at our door!
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1361244418
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@b4n approved this pull request. Apart the small doc changes, LGTM. Limited testing shows it works nicely, including mixed languages in a tags file. Older tags files I had laying around using either Pipe or Tagmanager format still seemed to work as usual. I didn't have any older ctags tags file with the now unsupported header, but adding it to a new one produces the warning yet seems to still work without too much problem. > + +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so to generate symbols for all sources in the my_project +directory one can use:: + +ctags -n --fields=EfiklsZSt -R -o my_project.c.tags my_project + +Additional options may be given to the ``ctags`` tool, for instance, to restrict +the generated tags file to some languages only, use certain tag kinds, etc. + +Note that when the ``l`` field (specifying the programming language) is enabled, +the language of all symbols is set based on the value of this field instead of +the language specified in the extension of the tags file. ```suggestion the language specified in the extension of the tags file. You however still have to name the file according to the same rules regardless of whether the language field is used. ``` or something like that, IIUC what you said somewhere else. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1680877018 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. bc49cd2750dacf0bb5f82b7f30d328df79f23e9e Document the recommended way of generating tags files -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/43ee190b8ff0886a4b55313a7f75ba0da4c6083d..bc49cd2750dacf0bb5f82b7f30d328df79f23e9e You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request.
> }
- else if (0 == strcmp(key, "file")) /* static (local)
tag */
- tag->local = TRUE;
- else if (0 == strcmp(key, "signature")) /* arglist */
+ else
+ g_array_append_val(unknown_fields, i);
+ }
+
+ if (!tag->scope)
+ {
+ /* search for scope introduced by scope kind name only
after going
+* through all extension fields and having tag->lang
updated when
+* "language" field is present */
+ for (idx = 0; !tag->scope && idx < unknown_fields->len;
idx++)
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request
see item 6
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1360524368
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request.
> }
- else if (0 == strcmp(key, "file")) /* static (local)
tag */
- tag->local = TRUE;
- else if (0 == strcmp(key, "signature")) /* arglist */
+ else
+ g_array_append_val(unknown_fields, i);
+ }
+
+ if (!tag->scope)
+ {
+ /* search for scope introduced by scope kind name only
after going
+* through all extension fields and having tag->lang
updated when
+* "language" field is present */
+ for (idx = 0; !tag->scope && idx < unknown_fields->len;
idx++)
> Or make this i, and make the local idx, which might be more idiomatic.
Yes, this is better, done this way.
By the way, how did you do these fancy committable "suggested change" thingies
at github?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1360465788
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. 43ee190b8ff0886a4b55313a7f75ba0da4c6083d Fix compiler warnings related to loop index variables -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/a088e452fbbef8bc08d0bc350265d98e45f048f0..43ee190b8ff0886a4b55313a7f75ba0da4c6083d You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request.
> {
- if ((NULL == fgets(buf, BUFSIZ, fp)) || ('\0' == *buf))
- return FALSE;
- }
- while (strncmp(buf, "!_TAG_", 6) == 0); /* skip !_TAG_ lines */
+ TMTagType type;
+ TMTag *tag;
+ guint i, idx;
h C99, better be careful, next it will be C11 ;-P
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1359986284
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. a088e452fbbef8bc08d0bc350265d98e45f048f0 Fix some compiler warnings -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/4ca132618802e4114c5ee08cbd8a7e55c8609e4c..a088e452fbbef8bc08d0bc350265d98e45f048f0 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@b4n requested changes on this pull request.
First pass only based on *code* review, no actual testing yet.
> + TMParserType lang =
> tm_ctags_get_named_lang(value);
+ if (lang >= 0)
+ tag->lang = lang;
```gcc
../../../src/tagmanager/tm_source_file.c: In function 'read_ctags_file':
../../../src/tagmanager/tm_source_file.c:394:46: warning: declaration of 'lang'
shadows a parameter [-Wshadow]
394 | TMParserType lang =
tm_ctags_get_named_lang(value);
| ^~~~
../../../src/tagmanager/tm_source_file.c:309:66: note: shadowed declaration is
here
309 | static void read_ctags_file(const gchar *tags_file, TMParserType lang,
GPtrArray *file_tags)
| ~^~~~
```
Maybe rename the local `tag_lang`?
```suggestion
TMParserType tag_lang =
tm_ctags_get_named_lang(value);
if (tag_lang >= 0)
tag->lang = tag_lang;
```
> {
- if ((NULL == fgets(buf, BUFSIZ, fp)) || ('\0' == *buf))
- return FALSE;
- }
- while (strncmp(buf, "!_TAG_", 6) == 0); /* skip !_TAG_ lines */
+ TMTagType type;
+ TMTag *tag;
+ guint i, idx;
```gcc
../../../src/tagmanager/tm_source_file.c:409:39: warning: declaration of 'i'
shadows a previous local [-Wshadow]
409 | guint i = g_array_index(unknown_fields,
guint, idx);
| ^
../../../src/tagmanager/tm_source_file.c:320:23: note: shadowed declaration is
here
320 | guint i, idx;
| ^
```
Easy solution is to declare those in the `for` initialization, as we now accept
this syntax.
```suggestion
```
> - {
- p = tab + 2;
- while (*p && *p != '\n' && *p != '\r')
+ tag = tm_tag_new();
+ tag->refcount = 1;
+ tag->name = g_strdup(entry.name);
+ tag->type = type;
+ tag->lang = lang;
+ tag->local = entry.fileScope; /* 'f' field */
+ tag->line = entry.address.lineNumber; /* 'n' field */
+ tag->file = NULL;
+
+ if (tm_parser_is_anon_name(lang, tag->name))
+ tag->flags |= tm_tag_flag_anon_t;
+
+ for (i = 0; i < entry.fields.count; i++)
```suggestion
for (guint i = 0; i < entry.fields.count; i++)
```
> }
- else if (0 == strcmp(key, "file")) /* static (local)
tag */
- tag->local = TRUE;
- else if (0 == strcmp(key, "signature")) /* arglist */
+ else
+ g_array_append_val(unknown_fields, i);
+ }
+
+ if (!tag->scope)
+ {
+ /* search for scope introduced by scope kind name only
after going
+* through all extension fields and having tag->lang
updated when
+* "language" field is present */
+ for (idx = 0; !tag->scope && idx < unknown_fields->len;
idx++)
```suggestion
for (guint idx = 0; !tag->scope && idx <
unknown_fields->len; idx++)
```
Or make this `i`, and make the local `idx`, which might be more idiomatic.
> + int j;
+ for (j = 0; lang_kinds[j]; j++)
…and while at it
```suggestion
for (gint j = 0; lang_kinds[j]; j++)
```
> {
- g_free(tag->arglist);
- tag->arglist = g_strdup(value);
+ guint i = g_array_index(unknown_fields, guint,
idx);
+ const gchar *key = entry.fields.list[i].key;
+ const gchar *value = entry.fields.list[i].value;
+ int j;
+ for (j = 0; lang_kinds[j]; j++)
+ {
+ gchar kind = lang_kinds[j];
```suggestion
const gchar kind = lang_kinds[j];
```
> {
- g_free(tag->arglist);
- tag->arglist = g_strdup(value);
+ guint i = g_array_index(unknown_fields, guint,
idx);
```suggestion
const guint i = g_array_index(unknown_fields,
guint, idx);
```
> {
- if (*end == ':' && ! value)
+
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany +directory, one can use:: + +ctags --fields=EfiklsZSt -o geany.c.tags -R geany + +Additional options may be given to the ``ctags`` tool, for instance, to restrict +the generated tags file to some languages only, use certain tag kinds, etc. + +Note that when the ``l`` field (specifying the programming language for every tag) +is enabled, the language of the tag is set based on this field instead of the Thanks, repushed with all the suggested changes. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1359877782 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. 4ca132618802e4114c5ee08cbd8a7e55c8609e4c Improve wording of geany.txt -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/bb64384a6af2631ff6f4168118ff27e0dff1c4b5..4ca132618802e4114c5ee08cbd8a7e55c8609e4c You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request. > +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany +directory, one can use:: + +ctags --fields=EfiklsZSt -o geany.c.tags -R geany + +Additional options may be given to the ``ctags`` tool, for instance, to restrict +the generated tags file to some languages only, use certain tag kinds, etc. + +Note that when the ``l`` field (specifying the programming language for every tag) +is enabled, the language of the tag is set based on this field instead of the Yep fine. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1359669807 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany +directory, one can use:: + +ctags --fields=EfiklsZSt -o geany.c.tags -R geany + +Additional options may be given to the ``ctags`` tool, for instance, to restrict +the generated tags file to some languages only, use certain tag kinds, etc. + +Note that when the ``l`` field (specifying the programming language for every tag) +is enabled, the language of the tag is set based on this field instead of the Oh, my response from yesterday disappeared somewhere. So once again - you mean whether to use the word "tag" or "symbol" here, right? I checked geany.txt and we only use "tags" in "tags file" but never talk about tags themselves so probably "symbol" is the correct one here. I've modified it to: ``` Note that when the ``l`` field (specifying the programming language) is enabled, the language of all symbols is set based on the value of this field instead of the language specified in the extension of the tags file. ``` Does it sound OK? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1359479477 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request. > @@ -1785,6 +1786,25 @@ This works basically the same as on other platforms:: "c:\program files\geany\bin\geany" -g c:\mytags.php.tags c:\code\somefile.php +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany Could be either way, but my preference would be comma before so only. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1358951020 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > case-sensitive. This helps Geany to read the file properly. If this line is missing, Geany tries to auto-detect the used format but this -might fail. +might fail. Tag files using the CTags format should be left unmodified in the OK. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1358256408 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > @@ -1689,12 +1689,13 @@ Global tags files can have three different formats: * Pipe-separated format * CTags format -The first line of global tags files should be a comment, introduced -by ``#`` followed by a space and a string like ``format=pipe``, -``format=ctags`` or ``format=tagmanager`` respectively, these are +For the tagmanager or pipe-separated format, the first line of global tag files +should be a comment, introduced by ``#`` followed by a space and a string like Will do (also not mine ;-). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1358256236 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > @@ -1785,6 +1786,25 @@ This works basically the same as on other platforms:: "c:\program files\geany\bin\geany" -g c:\mytags.php.tags c:\code\somefile.php +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany But there should still be comma after "so", right? I will also renamed the "geany" directory to something like "my_project" to avoid possible confusion with the geany binary. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1358254677 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr requested changes on this pull request. A couple of changes. > @@ -1785,6 +1786,25 @@ This works basically the same as on other platforms:: "c:\program files\geany\bin\geany" -g c:\mytags.php.tags c:\code\somefile.php +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany delete "for instance" and comma after "directory" > +Generating tags files using ctags +* +Geany supports loading tag files generated using the ``ctags`` command-line +tool from the universal-ctags project (https://github.com/universal-ctags/ctags). +Even though Geany should work with any ctags file, it is recommended to use +certain fields to give Geany some additional information. The recommended fields +are ``EfiklsZSt``, so, for instance, to generate tags for all sources in the geany +directory, one can use:: + +ctags --fields=EfiklsZSt -o geany.c.tags -R geany + +Additional options may be given to the ``ctags`` tool, for instance, to restrict +the generated tags file to some languages only, use certain tag kinds, etc. + +Note that when the ``l`` field (specifying the programming language for every tag) +is enabled, the language of the tag is set based on this field instead of the question, is it the tag or the symbol in Geany that gets the language specified? > @@ -1689,12 +1689,13 @@ Global tags files can have three different formats: * Pipe-separated format * CTags format -The first line of global tags files should be a comment, introduced -by ``#`` followed by a space and a string like ``format=pipe``, -``format=ctags`` or ``format=tagmanager`` respectively, these are +For the tagmanager or pipe-separated format, the first line of global tag files +should be a comment, introduced by ``#`` followed by a space and a string like delete "a string like", its not "like" it is exactly one or other IIUC > case-sensitive. This helps Geany to read the file properly. If this line is missing, Geany tries to auto-detect the used format but this -might fail. +might fail. Tag files using the CTags format should be left unmodified in the "used format" above s/b "format used" (I know its not your change, but might as well fix it) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1675440113 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 1 commit. bb64384a6af2631ff6f4168118ff27e0dff1c4b5 Update geany.txt with ctags usage for generating tag files -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/d945009828e3c36b366a42448b009ea66f784090..bb64384a6af2631ff6f4168118ff27e0dff1c4b5 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> At least we can easily change the PHP tags generation script when needed. But we still need to support user's tag files possibly generated by older ctags versions so it should stay there for some time even if we update the php parser. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1760381748 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 3 commits. e1ae976a7284d59f24b387825e967a2e6794a330 Add ctags 'readtags' library to allow us parse ctags files d364c30e7973ceaf387f8c64211d3980a462f90b Fix compilation when TM_DEBUG is defined d945009828e3c36b366a42448b009ea66f784090 Use the readtags library to parse ctags files -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/c5bfe6ace392f4f8c925de1fcb0e22efee94aed5..d945009828e3c36b366a42448b009ea66f784090 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> > We'll still need the "unknown" check to support older ctags binaries > > unfortunately. At least we can easily change the PHP tags generation script when needed. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1760373686 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> We'll still need the "unknown" check to support older ctags binaries > unfortunately. Sure, but it'll still make it more coherent. Unless we fear it'd break compatibility at some level… > OK to rebase this branch on top of master so it can be more easily tested > with the php and py tag files? I'd also squash the commits a bit. Sure, fine with me. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1760275309 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> Good catch. Maybe it'd be worth submitting an update to that parser to be > more in line with the convention as well. We'll still need the "unknown" check to support older ctags binaries unfortunately. OK to rebase this branch on top of master so it can be more easily tested with the php and py tag files? I'd also squash the commits a bit. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1760091651 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> > I wrote most of it, it ought to be perfect > > Oh, I'm syncing it in the wrong direction - I should have updated ctags with > your code :-). Meh, I also wrote some of that one, so either way works :grinning: > I discovered one more problem - all parsers use `typeref:typename` but I just > discovered after looking at Enrico's php tags file and at the php parser > afterwards that php uses `typeref:unknown` for typenames for some reason. So > I updated the code to allow `unknown` behind `typeref` too. Good catch. Maybe it'd be worth submitting an update to that parser to be more in line with the convention as well. > Maybe should I also grab the latest version of the libreadtags library for > this PR? I noticed you also submitted some patch to it in the spring. I didn't check the diff, but my latest changes there were not crucial, they fixed C stdlib usage, and some very theoretical issues handling some non-ASCII bytes. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1759182634 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
Maybe should I also grab the latest version of the libreadtags library for this PR? I noticed you also submitted some patch to it in the spring. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1758610973 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> I wrote most of it, it ought to be perfect Oh, I'm syncing it in the wrong direction - I should have updated ctags with your code :-). I discovered one more problem - all parsers use `typeref:typename` but I just discovered after looking at Enrico's php tags file and at the php parser afterwards that php uses `typeref:unknown` for typenames for some reason. So I updated the code to allow `unknown` behind `typeref` too. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1758608430 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request.
> {
- gchar buf[BUFSIZ];
- gchar *p, *tab;
+ tagEntry entry;
+ tagFile *f = tagsOpen(tags_file, NULL);
+ const gchar *lang_kinds = tm_ctags_get_lang_kinds(lang);
+ GPtrArray *unknown_fields = g_ptr_array_new_full(10, NULL);
The fundamental reason was that I never used GArray ;-). I've updated the code
with your suggestion.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1355824250
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request.
> + fclose(fp); /* the readtags library opens the file by itself */
+ read_ctags_file(tags_file, mode, file_tags);
> doesn't this happen?
You are absolutely right. When `format=ctags` is found in the file, I added
```
g_warning("# format=ctags directive is no longer supported; please remove it
from %s", tags_file);
```
I also updated the documentation because it advised to use `# format=ctags`
(hopefully nobody reads it ;-)
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#discussion_r1355823379
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > @@ -436,7 +443,6 @@ static TMTag *new_tag_from_tags_file(TMSourceFile *file, > FILE *fp, TMParserType result = init_tag_from_file_alt(tag, file, fp); break; case TM_FILE_FORMAT_CTAGS: - result = init_tag_from_file_ctags(tag, file, fp, mode); Done. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1355820502 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 4 commits. 49386fb8c70a80019ef5cd75ba21ec86305759b2 Warn about using legacy # format=ctags and update documentation a2cf693790c238faf7ba32aa18b478b6eaa77dc6 Add a debug warning a4a7941d7a0a82a1ebc6edb1b90566e2d3851c05 Replace GPtrArray with GArray c5bfe6ace392f4f8c925de1fcb0e22efee94aed5 Allow "unknown" in addition to "typename" behind the typeref field -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/5635539d55f069a06211854bc67f1db5d1679c69..c5bfe6ace392f4f8c925de1fcb0e22efee94aed5 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> I'm not sure how robust our parser is against invalid input I wrote most of it, it ought to be perfect :grin: > or updated ctags file format In theory the format is perfectly well defined and stable (just adding new extension fields), so it should be fine™. But in practice uctags added support for escape sequences, which our parser doesn't understand, so it isn't. > so better use the upstream version of the official library to parse ctags > files so we don't have to worry about this. Yeah, it's a very large diff, but I guess using the canonical implementation is worth the extra copy. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1756418866 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@b4n commented on this pull request.
> @@ -436,7 +443,6 @@ static TMTag *new_tag_from_tags_file(TMSourceFile *file,
> FILE *fp, TMParserType
result = init_tag_from_file_alt(tag, file, fp);
break;
case TM_FILE_FORMAT_CTAGS:
- result = init_tag_from_file_ctags(tag, file, fp, mode);
might be worth a `g_warn_if_reached()`
> + fclose(fp); /* the readtags library opens the file by itself */
+ read_ctags_file(tags_file, mode, file_tags);
Doesn't the `# format=ctags` confuse this parser? I didn't try this yet, but
it feels like it should:
* prevent pseudo tags (`!_…` ones) from being recognized as such
* get parsed as a normal entry
doesn't this happen?
Agreed this header it not necessary (and actually with supporting detection
with some pseudo-tags probably not advisable), but it should either work, or we
shouldn't pretend it still does.
> {
- gchar buf[BUFSIZ];
- gchar *p, *tab;
+ tagEntry entry;
+ tagFile *f = tagsOpen(tags_file, NULL);
+ const gchar *lang_kinds = tm_ctags_get_lang_kinds(lang);
+ GPtrArray *unknown_fields = g_ptr_array_new_full(10, NULL);
any reason not to use `GArray` instead? OK it's a tad awkward as well, but
less so than `GPtrArray` for storing integers I'd say. I don't have to draw
you a picture, but:
```c
GArray *array = g_array_sized_new(FALSE, FALSE, sizeof(guint), 10);
g_array_append_val(array, i);
guint i2 = g_array_index(array, guint, idx);
```
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#pullrequestreview-1669177204
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > - /* tag name */ - if (! (tab = strchr(p, '\t')) || p == tab) - return FALSE; - tag->name = g_strndup(p, (gsize)(tab - p)); - p = tab + 1; + if (entry.kind[0] && entry.kind[1]) + type = tm_parser_get_tag_type(tm_ctags_get_kind_from_name(entry.kind, lang), lang); // 'K' field Done, thanks. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1353468940 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee pushed 2 commits. 14c3b8081c79da98f053eda3a2f61925eafa5933 Use C-style comments 5635539d55f069a06211854bc67f1db5d1679c69 End both loops when scope found -- View it on GitHub: https://github.com/geany/geany/pull/3049/files/f49b19f3bfe61cf7ae27075f28ec5bd7c570a1ba..5635539d55f069a06211854bc67f1db5d1679c69 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@techee commented on this pull request. > @@ -154,7 +154,7 @@ gboolean tm_tag_is_anon(const TMTag *tag); const char *tm_tag_type_name(const TMTag *tag); -TMTagType tm_tag_name_type(const char* tag_name); +TMTagType tm_tag_name_type(const char* tag_name, TMParserType lang); Yes. This change is quite independent from this PR, I just needed to define TM_DEBUG for debugging this PR so this is why it's here. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1350713075 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request. > @@ -154,7 +154,7 @@ gboolean tm_tag_is_anon(const TMTag *tag); const char *tm_tag_type_name(const TMTag *tag); -TMTagType tm_tag_name_type(const char* tag_name); +TMTagType tm_tag_name_type(const char* tag_name, TMParserType lang); Reference https://github.com/geany/geany/blob/987cc3f106bf1eb6c0c0667bf5f24582022180ae/src/tagmanager/tm_tag.c#L692 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1350082276 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@elextr commented on this pull request. > @@ -154,7 +154,7 @@ gboolean tm_tag_is_anon(const TMTag *tag); const char *tm_tag_type_name(const TMTag *tag); -TMTagType tm_tag_name_type(const char* tag_name); +TMTagType tm_tag_name_type(const char* tag_name, TMParserType lang); No change, its fixing the .h to match the existing .c -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#discussion_r1350081092 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@kugel- requested changes on this pull request. > - /* tag name */ - if (! (tab = strchr(p, '\t')) || p == tab) - return FALSE; - tag->name = g_strndup(p, (gsize)(tab - p)); - p = tab + 1; + if (entry.kind[0] && entry.kind[1]) + type = tm_parser_get_tag_type(tm_ctags_get_kind_from_name(entry.kind, lang), lang); // 'K' field C-style comments, please : `/* 'K' field */` (more of that below) > @@ -154,7 +154,7 @@ gboolean tm_tag_is_anon(const TMTag *tag); const char *tm_tag_type_name(const TMTag *tag); -TMTagType tm_tag_name_type(const char* tag_name); +TMTagType tm_tag_name_type(const char* tag_name, TMParserType lang); can't find the corresponding change in the C file- -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1664057269 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
I wonder why PRs like this are open or so long and then rushed into the release. Not enough pinging? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1752640119 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
By the way, I really think this PR should go to 2.0. @eht16 converted various tag-generating scripts to use the ctags file format and some things aren't parsed correctly now without this patch, see the end of https://github.com/geany/geany/pull/3512#issue-1740467282. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1752609067 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
> Now conflicts are fixed will try to test in the next few days. IIRC it seemed to work with limited testing way back then, but seems I didn't post that, sorry. Didn't have anything complicated to test with. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1751545334 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
Should I rebase this PR on top of master and squash some of the commits? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1750772060 You are receiving this because you are subscribed to this thread. Message ID:
