@b4n commented on this pull request.


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 don't know how expansive it'd be indeed, so whether or not it's reasonable to do it, but we could check.
From a quick look, it'll perform a linear search for the lexer of that name, and call Create() on the module, which either calls the lexer's LexerFactoryFunction (calling lexer code, which normally would instantiate an instance of the relevant lexer), or instantiate a new LexerSimple. So… it seems to be in between cheap and expansive, but I didn't profile anything.

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.

I was wondering whether, now that we use a name for the lexers, if we'd want to avoid hard-coding things in highlightingmappings.h and move that to the filetypes files à-la-SciTE? E.g. maybe something like this:

[lexer]
# could also be e.g. in [settings] as lexer_name
name = cpp
# style[lexer style number] = style name for use in [styling]
styles[0] = default
styles[1] = comment
styles[2] = commentline
styles[3] = commentdoc
# …
# keywors, those could possibly use GetWordListDescription() or similar to have actual names
keywords[0] = primary
keywords[1] = secondary
keywords[2] = docComment
# …

or if we were to break compatibility, we could avoid adding a proxy and just use the style/keyword numbers directly instead of mapping them twice..

I don't particularly like the idea of having arbitrary numbers there, but in the end, is it worse than the internal tables? And maybe we could use NamedStyles()/NameOfStyle() and DescribeWordListSets() if enough lexers we need support that.

Anyway, it might not be relevant as we're not loading all filetype files anyway.

But note that any list to check would have to actually be synchronized with what is used; the issue in #3616 would only be found if checking the ID we used, not the list of lexers we had, as it was missing from the AddLexerModules() call.


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/1711282555@github.com>