@techee commented on this pull request.
Thumbs up for fixing the only warning that we, ordinary humans with no special
compiler options, see during Geany compilation :-).
Looks good _but_ I didn't check if the actual lexer names are correct - but see
the related comment in the review.
> @@ -696,23 +696,24 @@ gint sci_get_lexer(ScintillaObject *sci)
}
-void sci_set_lexer(ScintillaObject *sci, guint lexer_id)
+void sci_set_lexer_name(ScintillaObject *sci, const gchar *lexer_name)
This sounds like the function sets the name of the lexer but it just sets it
based on the name. Wouldn't it be better to keep it named as before?
> {
- g_warning("Failed to find lexer for ID %u", lexer_id);
+ g_warning("Failed to find lexer for name %s", lexer_name);
I was wondering also in https://github.com/geany/geany/pull/3616 if it would be
possible to do something similar like we do for the TM mappings checks - when
starting Geany, going through all the lexers we use and check if we manage to
create them; if not, hard-crash with `g_error()`.
This would make errors like https://github.com/geany/geany/pull/3616
impossible. But this depends on two things:
1. `CreateLexer()` shouldn't be too expensive to call, I haven't checked what
exactly it does.
2. We'd need to iterate through all lexers. Right now all lexers have to be
mentioned explicitly at 2 different places in switches in `highlighting.c` and
we probably don't want to add one more. So we'd have to create a table
containing all the lexers, possibly in a similar way like in `filetypes.c`. As
a side-effect one could then maybe avoid the warning from
https://github.com/geany/geany/pull/2398 (which is produced by clang by
default).
Thoughts?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3668#pullrequestreview-1711028659
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3668/review/[email protected]>
