Initial support for Scintilla sub-styles #3794
Conversation
src/highlighting.c
Outdated
| /* FIXME: this is sub-stylable, but we can't figure out the base | ||
| * style without the Scintilla instance -- and we can't simply | ||
| * assume that if the style ID is larger than the max possible | ||
| * one it's this, because SCE_C_IDENTIFIER is also sub-stylable... */ |
There was a problem hiding this comment.
This I don't see how to fix short of having a separate API taking a ScintillaObject instead of a lexer ID. We might be able to store info about the allocated sub-style somewhere that could be accessed from here if we could rely on sub-style allocation to give predictable values. This would require Scintilla to actually be deterministic about that, which it might be, but likely not if some calls could happen adding or modifying substyles (think of a plugin that would mess with this).
There was a problem hiding this comment.
Should it actually be per Scintilla instance, the plugin might only mess with one?
There was a problem hiding this comment.
It is, so yeah, we'd have to be sure allocation is the same for all lexer instances. That's the tricky part, then we can just fill something when initially allocating the substyles.
The proper solution should be style = SCI_GETSTYLEFROMSUBSTYLE(style) but we can't do that with this API.
There was a problem hiding this comment.
Also, don't we only instantiate one lexer per sci instance? So the whole process needs to be done every time a new tab with a new filetype is opened because there is now a new type of lexer to query?
There was a problem hiding this comment.
Yes we do have one lever per instance, that's why we'd need the allocations to be the same for all of them for this to have a chance of working.
But really, I probably should look at callers for this to see if anybody actually doesn't have a Scintilla instance ready at hand when calling this.
There was a problem hiding this comment.
Yeah, it should only be called on a document, so there should be a SCI available. Otherwise how did they get the "style" to pass in???
There was a problem hiding this comment.
Fixed in e9ca102 by introducing an alternate API and deprecating this
A filetype file can define substyles for appropriate styles using a
`stylename.N=style` syntax in both `styling` and `keywords` sections:
[styling]
# ...
identifier.1=some_named_style
[keywords]
# ...
identifier.1=foo bar baz
This allows to add arbitrarily more keyword styles for lexers that
support the feature (C, Python, Bash, GDScript and Verilog currently).
Use the newer SCI_GETSTYLEINDEXAT which supports styles > 127 instead of the historic SCI_GETSTYLEAT that is limited to 127. This is useful if a lexer has more than 127 styles, which is notably the case when using the sub-styles feature.
Display them as `BASE "." SUB_INDEX` (e.g. `11.1` for the first sub-style of style 11).
highlighting_is_*_style() API cannot deal with substyles, because they are inherently dynamic, but the API of these functions don't have access to a mean of fetching the required dynamic info. So, introduce replacement API editor_is_*_a(), and deprecate the other one. The new API takes an editor and a position, which allows to retrieve the required dynamic information, and leaves room to further extend the implementation if need arises. Some design choices: Q: Why use a GeanyEditor rather than a ScintillaObject? A: We don't currently have original APIs that take a ScintillaObject. The APIs we have that do take one are mere wrapper around one or a few Scintilla calls, not original APIs altogether. This means that one that do doesn't really have a spot to reside in, both in naming and source location. Additionally, this could allow future addition to the API that require extra data held on the GeanyEditor instance. The only downside is that this doesn't necessarily cover custom Scintilla instances, but it's a rather odd use case of having one and requiring this kind of info from it. Q: Why replace style with position? Current implementation would work fine with style as well. A: Because of flexibility, and current usage. Flexibility: using a position could allow for looking around in the future if the exact meaning of a style could depend on the ones around, or if Scintilla had extra separate info that could be useful for determining the category that is not tied to the style itself. Reviewing current usage suggests that all callers perform a slight variation on `highlighting_is_*_style(lexer, style_at(sci, pos))`, so accepting a position directly just makes the API easier to use. The very few callers that use the style separately as well merely suffer from one additional call in this situation. Finally, there is no reason to tie the question "is there a comment here?" to a style from an user standpoint, so requiring one simply leak some internals when answering this question.
Following the deprecation of the former in favor of the latter.
This might not be entirely thorough, yet should cover most ground by reviewing all `sci_get_style_at()` calls which should be the primary source of Scintilla styles.
This is a first attempt with little testing, but it seems to work pretty OK. However, please review all levels, from design to implementation.
To summarize, what this does is look for
stylename.Nfor styles that are known to be sub-stylable1, as well as the corresponding keywords/identifiers2. From there, it's just regular styles one can map as usual.One limitation I am already aware of is that as it's not regular keywords, it doesn't integrate with the code merging the ctags symbols into them. It could be possible to augment this to work, but with the current design this would need a way of representing this in the filetypes file (which would be good, but is not the case currently).
One arbitrary design choice was to use the syntax
stylename.N. This is mostly because usingstylename[i]is tricky/hacky usingGKeyFile, because it looks similar to Scintilla's properties, and doesn't clash with any style name. Other suggestions are welcome.Footnotes
we unfortunately cannot use
SCI_GETSUBSTYLEBASESbecause we don't have access to a Scintilla instance when loading style data. Hence adding a new field inHLStyle. ↩Note here that this feature is styles-based in Scintilla, meaning that there is no regular keywords entry associated, it's defined as kind of a style property. ↩