A bunch of string and comment styles were missing and so Geany could not detect the corresponding styles in code.
Compared the mappings in `src/highlighting.c` against the list of used Scintilla lexers in `scintilla/src/Catalogue.cxx` and added missing style mappings. I also added "stubs" for lexers which don't support string or style, just to have them listed and make sure they are not missing again.
Another pair of eyes to double check copy&paste mistakes are welcome.
Missing styles were detected in https://github.com/geany/geany-plugins/commit/236363f0e7dfab7db8690cc051a534.... You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1502
-- Commit Summary --
* Add missing string and comment styles for various lexers
-- File Changes --
M src/highlighting.c (86)
-- Patch Links --
https://github.com/geany/geany/pull/1502.patch https://github.com/geany/geany/pull/1502.diff
Generally LGBI, but I'm only part way through, comments so far below, will go further when I have time.
What about SCE_C_USERLITERAL its a type of string.
Also SCE_RB_STRING_QR and SCE_RB_STRING_QW
What about REGEX types, they are usually a type of string, particularly SCE_C_REGEX but there are also SCE_HJ_REGEX, SCE_HJA_REGEX, SCE_PL_REGEX.
Do you have examples for the mentioned styles (except regex, they are easy enough :D). I have no clue what `SCE_C_USERLITERAL` is.
`SCE_RB_STRING_QR` and `SCE_RB_STRING_QW` is probably safe to add, yes. Whatever QR and QW might mean.
I agree on adding regex styles. Will do with the rest once agreed.
I have no clue what SCE_C_USERLITERAL is.
Its a C++ thing :)
A class can define a suffix to go after a literal to make it define an instance of the class instead of the type the literal usually makes. For example the `std::string` literal `"...."s` is actually just a user defined literal, not a built-in literal like the `"..."` char array literal is. For gory details see [here](http://en.cppreference.com/w/cpp/language/user_literal).
Unfortunately as that link above shows, it also applies to char and numeric literals, and Scintilla doesn't distinguish between them, dammit. But given that `std::string` literals are far more common than numeric literals (in my experience) I would still add it to strings until someone complains, then prod Neil with a patch to separate string user literals from the others.
Whatever QR and QW might mean.
Dunno, but there are other languages with them already added IIRC.
What about REGEX types, they are usually a type of string, particularly SCE_C_REGEX but there are also SCE_HJ_REGEX, SCE_HJA_REGEX, SCE_PL_REGEX.
Are they strings? I agree it's not code, but I'm not quite sure it should be string; it's rather another type of syntax than arbitrary data like strings are. For example, it could make sense to have e.g. spellcheck use `highlighting_is_string_style() || highlighting_is_comment_style()`, which could be different from `!highlighting_is_code_style()` (e.g. I think pre-processor qualifies as a difference in this case).
@b4n, yeah, but autocomplete should not occur inside regexes as well as comments and strings, I think there are multiple use-cases for these predicates and we gotta be sure which one its meant to support.
@eht16 pushed 1 commit.
aca4e1f Add more string styles
I just added SCE_RB_STRING_Q* and SCE_C_USERLITERAL.
What do we want to about the regex styles? Considering those styles as strings would "break" SpellCheck (provided we change it to use `highlighting_is_*_style()` instead of its own mapping). On the other hand, it could help the AutoClose plugin which already uses `highlighting_is_code_style` to check wether it should close brackets or not and auto-closing brackets in regelar expressions might be confusing.
When SpellCheck would use `highlighting_is_*_style()` for style checking, it could be OK to add a successor check to treat the regex styles special for the SpellCheck context.
Irrespective of the uses in plugins, Geany core uses `highlighting_is_code_style()` for:
* matching parentheses, definitely don't want to do that in regexes * brace indenting, don't want to see braces in regexes * get line end position, hmm this might want to see regexes as code, not sure * parens in calltip handling, don't want to match in regexes * enabling autocomplete (so it doesn't happen in strings and comments) and that should exclude regexes as well
That seems to suggest that in core most places regexes should not be counted as code. My simple solution was to put them in `highlighting_is_string_style()` since it already has the basic infrastructure, but I understand that upsets spell checking and maybe other uses of `highlighting_is_string_style()` so I guess that says we need to add them to `highlighting_is_code_style()` or have a `highlighting_is_regex_style()` that it can use.
We don't need to take care about SpellCheck right now as it doesn't use any `highlighting_is_*_style()` functionality yet, i.e. I need to touch the SpellCheck anyway in this regard.
Thanks for the great list of uses of `highlighting_is_code_style()` which is convincing to add regex styles to `highlighting_is_string_style()`.
Will do if @b4n and @elextr will agree.
@eht16 pushed 1 commit.
39b6280 Add regex styles as string styles
I added the regex styles.
Not completely sure about `SCE_PL_REGEX_VAR`, it doesn't seem to be used in LexPerl.cxx, so hard to say what it is meant for.
Not completely sure about SCE_PL_REGEX_VAR, it doesn't seem to be used in LexPerl.cxx, so hard to say what it is meant for.
Might be a leftover from a previous version, its tested against, but never set.
I'd like to merge this soon so that we get it tested on master and ideally will fit for 1.32. OK?
Looks fine to me.
While I agree that regex shouldn't be considered regular code, I think it might also not be a good idea to consider it a string for all the mentioned reasons (different semantics basically). Without actually looking at all the code and uses, I would still think we should rather special-case it as something that doesn't match string, comment nor code using those functions.
@b4n, thats a very different PR to this since it means all users of `is_string_style()` need to add extra tests for `is_regex_style()`.
Whilst I agree (see above) thats probably the "correct" solution, adding regexen to string styles is a hack for now when resources to do the right thing are not available.
Merged #1502.
Added to gain experience with it.
github-comments@lists.geany.org