What does upstream say about the changes?

What is the setting set to if the `tag_parser=` is not present? And can `tag_parser=` mean that?

For most languages the parser is hard-coded using the table here: ... I think only a few non-builtin filetypes like JSON use the tag_parser option to specify the parser by name. But if the name is invalid or not provided, we shouldn't crash which this PR tries to do.

And also the hard-coded parsers can be overridden by this option so by specifying e.g. `tag_parser=Python` for the C filetype, we'd start using the Python parser for it (doesn't make much sense in this case). So by specifying `tag_parser=` we override the C parser to "no parser" so TM is completely disabled for C and LSP doesn't interfere with it.

Sorry, I wasn't clear, many of the custom filetypes have no parser and don't crash. So what do they set the tag_parser to?

The crash only happened because Geany was trying to load the global tags file for Python (`data/tags/std.py.tags`) and it was Python for which I set `tag_parser=`. The custom filetypes that don't use `tag_parser` also set the parser to `TM_PARSER_NONE` but don't have any global tags file so the crash doesn't happen for them.

I could have also fixed this particular crash by checking the parser for `TM_PARSER_NONE` inside e.g. `tm_workspace_load_global_tags ()` but I think it's a good idea to protect all the ctags-interfacing functions against this value.

And can tag_parser= be set to the same value that successfully works for custom filetypes.

Well, I think we should be able to allow users disable some ctags parser if it e.g. causes crashes and `tag_parser=` is the way to do it currently. And setting the parser of say the Pascal filetype to the C parser isn't a good idea as the C parser will be constantly confused by Pascal sources.

So the question goes back to, since the proposed changes are to ctags will upstream accept the changes so we don't have to keep a patch to ctags?

See the first answer here: https://github.com/geany/geany/pull/3865#issuecomment-2094775899 :-). The file `tm_ctags.c` is our source that hides the ctags "API" behind some thin layer and it is only this file that includes ctags headers so we don't pollute the rest of our sources with symbols included from ctags.

@b4n commented on this pull request.

LGTM, apart possibly the inline comments. Sad we have to do that in several functions, but makes sense.

• kindDefinition *def;

+ + if (lang == TM_PARSER_NONE) + return "unknown"; + + def = getLanguageKindForLetter(lang, kind);

Maybe something like this to avoid repeating the fallback value? ```suggestion kindDefinition *def = NULL;

if (lang != TM_PARSER_NONE) def = getLanguageKindForLetter(lang, kind); ```

• kindDefinition *def;

+ + if (lang == TM_PARSER_NONE) + return '-'; + + def = getLanguageKindForName(lang, name);

Ditto here, maybe:

```suggestion kindDefinition *def = NULL;

if (lang != TM_PARSER_NONE) def = getLanguageKindForName(lang, name); ```

@techee commented on this pull request.

• kindDefinition *def;

+ + if (lang == TM_PARSER_NONE) + return "unknown"; + + def = getLanguageKindForLetter(lang, kind);

Yes, that's better - done at both places.

@b4n approved this pull request.