Merged #3865 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3865#event-12947445320
You are receiving this because you are subscribed to this thread.
Message ID:
@b4n approved this pull request.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3865#pullrequestreview-2079821073
You are receiving this because you are subscribed to this thread.
Message ID:
> LGTM, apart possibly the inline comments.
I've made the changes, amended the previous commit with it and force-pushed the
result.
> Sad we have to do that in several functions, but makes sense.
The crash itself would probably not require all the places but better to have
it handled everywher
@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.
--
Reply to this email directly or view it on GitHub:
ht
@techee pushed 1 commit.
dcd46bbe62e32d3ed1f6f25b977cdfeecd95ee2d Protect tm_ctags_*() functions
against TM_PARSER_NONE language parameter
--
View it on GitHub:
https://github.com/geany/geany/pull/3865/files/5bb09fbd2a0a6aa4d4fec8a187e3b4f171cdbe0e..dcd46bbe62e32d3ed1f6f25b977cdfeecd95ee2d
You
@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);
> 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.
Too easy then.
--
Reply to this email directly or view it on GitHub:
http
> 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 t
> 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.
Ok, it didn't make sense that TM_PARSER_NONE worked for custom, but not
built-in, but in fact that we are doing something that jus
> 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
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? And can `tag_parser=` be set
to the same value that successfully works for custom filetypes.
--
Reply to this email directly or view it on GitHub:
https://github.com/ge
> 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-co
> What does upstream say about the changes?
The change is in our sources, not the upstream ones.
> What is the setting set to if the tag_parser= is not present?
For most languages the parser is hard-coded using the table here:
https://github.com/geany/geany/blob/ef2255bced523a3ad75773bac3e77b02
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?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3865#issuecomment-2094556791
You are receiving this because you a
14 matches
Mail list logo