@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.


In src/sciwrappers.c:

> @@ -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?


In src/sciwrappers.c:

>  	{
-		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 #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 #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 #2398 (which is produced by clang by default).

Thoughts?


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3668/review/1711028659@github.com>