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.N` for styles that are known to be sub-stylable[^1], as well as the corresponding keywords/identifiers[^2]. 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 using `stylename[i]` is tricky/hacky using `GKeyFile`, because it looks similar to Scintilla's properties, and doesn't clash with any style name. Other suggestions are welcome.
[^1]: we unfortunately cannot use [`SCI_GETSUBSTYLEBASES`](https://scintilla.org/ScintillaDoc.html#SCI_GETSUBSTYLEBASES) because we don't have access to a Scintilla instance when loading style data. Hence adding a new field in `HLStyle`. [^2]: 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. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3794
-- Commit Summary --
* Initial support for Scintilla sub-styles * DO NOT MERGE: Add sample sub-styles for Python * Make sci_get_style_at() work with larger number of styles * Display sub-styles in the statusbar %Y placeholder
-- File Changes --
M data/filedefs/filetypes.python.in (7) M src/highlighting.c (111) M src/highlightingmappings.h (1897) M src/sciwrappers.c (2) M src/ui_utils.c (14)
-- Patch Links --
https://github.com/geany/geany/pull/3794.patch https://github.com/geany/geany/pull/3794.diff
@b4n commented on this pull request.
@@ -877,6 +939,21 @@ static void styleset_from_mapping(ScintillaObject *sci, guint ft_id, guint lexer
if (styles[i].fill_eol) SSM(sci, SCI_STYLESETEOLFILLED, styles[i].style, TRUE); set_sci_style(sci, styles[i].style, ft_id, i); + if (styles[i].sub_stylable) + { + LexerStyle *style = &style_sets[ft_id].styles[i]; + + if (style->sub.count) + { + /* FIXME: can substyle allocation fail? and how do we know? */
Here I probably just need to read Lexilla's sources to see what happens, as it's not documented. Maybe it always works, maybe it returns a negative value?
@b4n commented on this pull request.
/* 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... */
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).
@b4n commented on this pull request.
g_string_append_c(stats_str, ' ');
- g_string_append_printf(stats_str, "%d", - sci_get_style_at(doc->editor->sci, pos)); + g_string_append_printf(stats_str, "%d", base_style); + if (base_style != style) + { + gint sub_start = scintilla_send_message(doc->editor->sci, SCI_GETSUBSTYLESSTART, base_style, 0); + + g_string_append_printf(stats_str, "+%d", style + 1 - sub_start);
Here the format is also pretty much whatever I first though of, and `B.S` might be better than `B+S`.
@elextr commented on this pull request.
/* 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... */
Should it actually be per Scintilla instance, the plugin might only mess with one?
@b4n commented on this pull request.
/* 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... */
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.
@elextr commented on this pull request.
/* 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... */
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?
@nyamatongwe commented on this pull request.
@@ -877,6 +939,21 @@ static void styleset_from_mapping(ScintillaObject *sci, guint ft_id, guint lexer
if (styles[i].fill_eol) SSM(sci, SCI_STYLESETEOLFILLED, styles[i].style, TRUE); set_sci_style(sci, styles[i].style, ft_id, i); + if (styles[i].sub_stylable) + { + LexerStyle *style = &style_sets[ft_id].styles[i]; + + if (style->sub.count) + { + /* FIXME: can substyle allocation fail? and how do we know? */
Lexers have a limited number of substyles since they are styles and styles are 8-bit values. Since lexers use styles for other purposes, lexers that support substyles commonly support 64. `SCI_ALLOCATESUBSTYLES` returns a negative number when it fails such as exhausting available substyles. Currently failure only returns -1, but there could be other error codes in the future. Added to documentation with https://sourceforge.net/p/scintilla/code/ci/372e3958c9ce05f59012ca695d8679d0...
@b4n commented on this pull request.
/* 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... */
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.
@b4n commented on this pull request.
@@ -877,6 +939,21 @@ static void styleset_from_mapping(ScintillaObject *sci, guint ft_id, guint lexer
if (styles[i].fill_eol) SSM(sci, SCI_STYLESETEOLFILLED, styles[i].style, TRUE); set_sci_style(sci, styles[i].style, ft_id, i); + if (styles[i].sub_stylable) + { + LexerStyle *style = &style_sets[ft_id].styles[i]; + + if (style->sub.count) + { + /* FIXME: can substyle allocation fail? and how do we know? */
@nyamatongwe great, thanks for the details and the updated docs!
@b4n commented on this pull request.
@@ -877,6 +939,21 @@ static void styleset_from_mapping(ScintillaObject *sci, guint ft_id, guint lexer
if (styles[i].fill_eol) SSM(sci, SCI_STYLESETEOLFILLED, styles[i].style, TRUE); set_sci_style(sci, styles[i].style, ft_id, i); + if (styles[i].sub_stylable) + { + LexerStyle *style = &style_sets[ft_id].styles[i]; + + if (style->sub.count) + { + /* FIXME: can substyle allocation fail? and how do we know? */
This should be fixed now.
@b4n commented on this pull request.
g_string_append_c(stats_str, ' ');
- g_string_append_printf(stats_str, "%d", - sci_get_style_at(doc->editor->sci, pos)); + g_string_append_printf(stats_str, "%d", base_style); + if (base_style != style) + { + gint sub_start = scintilla_send_message(doc->editor->sci, SCI_GETSUBSTYLESSTART, base_style, 0); + + g_string_append_printf(stats_str, "+%d", style + 1 - sub_start);
I changed my mind and implemented `B.S`. Reasoning is that it looks more similar than defining the style, and has " style-and-a-half vibe" rather than an odd addition. But that's still quite arbitrary, feel free to suggest something even better :)
@elextr commented on this pull request.
/* 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... */
Yeah, it should only be called on a document, so there should be a SCI available.
@elextr commented on this pull request.
g_string_append_c(stats_str, ' ');
- g_string_append_printf(stats_str, "%d", - sci_get_style_at(doc->editor->sci, pos)); + g_string_append_printf(stats_str, "%d", base_style); + if (base_style != style) + { + gint sub_start = scintilla_send_message(doc->editor->sci, SCI_GETSUBSTYLESSTART, base_style, 0); + + g_string_append_printf(stats_str, "+%d", style + 1 - sub_start);
Yeah a.b is common subobject referencing syntax and here its referencing the substyle.
@b4n pushed 8 commits.
90ad258b26d550578c29f1802348a8bfb60d68af Add sample sub-styles for reference 24820c087216e29af99749a174ce78effde3957c Make sci_get_style_at() work with larger number of styles 555f0fd9b397e280e1d00080aadab91b98e4a105 Display sub-styles in the statusbar %Y placeholder 668fca10102e5fb51ce6ea55d1ae6981f8dc68e0 Add new Scintilla wrappers for substyles 4c9d4f88f9ee13be15bb8fc1caa1c3b259f72889 Deprecate highlighting_is_*_style() in favor of editor_is_*_at() 8b909ae20fe4bf5656deac7bf74001937573b232 Replace calls to highlighting_is_*_style() with editor_is_*_at() 86b95e530591fd5ffc6d6765bea1bd7beb63b39a Properly handle substyles throughout the code bf160f62ed27407a6db53c2f5b6fe1a7ca9d237c Update documentation to mention sub-styles
@b4n pushed 4 commits.
e9ca1025d49025b2eff35f63d97f098eb0955c48 Deprecate highlighting_is_*_style() in favor of editor_is_*_at() 5090cdd584491aef1e81b35305757332c3f68e87 Replace calls to highlighting_is_*_style() with editor_is_*_at() 06ff6c9e98f382e532a85cc318b294ec11a4ede2 Properly handle substyles throughout the code bbf676479127c9aa9dcf3e743ec6421b640c32d6 Update documentation to mention sub-styles
@b4n commented on this pull request.
/* 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... */
Fixed in e9ca1025d by introducing an alternate API and deprecating this
github-comments@lists.geany.org