-
Notifications
You must be signed in to change notification settings - Fork 698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Style Identifiers #284
Comments
Other editors have a concept of markers for general subsections of text that should move consistently with buffer manipulation. Style spans fit into that category as well. So this might be a good point to think about:
|
Are you talking about the plugin <-> core protocol or about the front-end <-> core protocol? Might be good to clarify that. For the front-end <-> core protocol I think sending the colors as RGBA value is not a problem. |
These are good questions to think about, but I'll quickly echo what @eyelash has said. My current thinking is that plugins communicate scopes to the core, the core applies a theme and resolves those into font/color styles, and the core communicates those styles to the front-end. I do see a numeric id system for plugin->core communication, strictly for efficiency. This lets us implement theme resolution as a simple id->id map in the hit case. @AljoschaMeyer I haven't thought too much about markers yet but I would guess they would work as a type of span. My idea is that spans are a fairly general concept, and would also incorporate annotations such as warnings, not just syntax highlighting. |
A variation, based on how Sublime works:
|
I think this makes sense generally and also makes conflict resolution potentially easier, as scopes have priorities implied in their hierarchy, and certain scopes like One side effect of this approach is that the concept of scopes & themes has to live in xi-core. This is a fairly big change, and is maybe an argument for moving syntax highlighting itself into core? It would definitely be convenient if we could reuse the scope/theme stuff already implemented in syntect, and if syntect is going to be a dependency keeping syntax highlighting in-process might make sense. |
@cmyr an approach @raphlinus and I discussed a while ago is splitting This is good because it maintains the property of having syntax parsing be a plugin, allowing fancier or faster parsing systems than syntect. It's also good because now other plugins can access the analysis that parsing plugins provide in order to do things like "expand to scope" and "go to function" regardless of the parsing plugin by taking advantage of a common intermediate representation. |
@trishume at first blush that sounds like a good idea to me. |
Couldn't a plugin handle theming, scopes and syntax detection? I don't see why the core should be opiniated about this stuff. Is the performance gain significant here? Aside: Obviously, this would require better inter-plugin communication. Which would be a lot of effort to implement. I'm feeling somewhat bad for just throwing in these suggestions without contributing any code. But I'm very excited about what Xi could become, so I'm giving my idealistic comments anyways. And when pragmatic requirements outweight idealism, that's still fine. I'm thankful for all of you investing your time into the project - the progress you made in the last few weeks is amazing. |
Your feedback is totally welcome, glad somebody is noticing we're getting some work done. (also glad, perhaps, that it isn't too many people... 😉) If I'm understanding: the main thing that core needs to be opinionated about is ensuring UI consistency. In the simplest case, this means that plugins should not be sending specific color information. There are other approaches, but to me they seem to add a bunch of complexity and not offer much upside, outside of some currently-very-abstract stuff like "improved flexibility". Having color schemes live in core, and having plugins send references to styles that are resolved by lookups into a color scheme, feels like a good solution for now. Plugins are still responsible for scopes and syntax detection, just not for assigning colors to them. |
Thanks for all the comments!
@AljoschaMeyer: Thanks for the link! Markers look similar to spans (but I only skimmed through the linked page). The concept of
@eyelash: I was actually thinking about providing the same approach for both plugin <-> core and front-end <-> core. I agree that we could keep the current approach for front-end <-> core. Would this require functionality to cleanup the frontend style cache? E.g. when switching between themes, the front-end might end up with a lot of unused styles.
@cmyr Maybe not a requirement? I am just thinking out loud now. I think it is not necessary that scopes live in the core. The core could just maintains a
@trishume Sounds good.
This requires to attach some semantic to a scope. In this case my thinking above (1) would not work, because there scope names are just "stupid" strings.
@AljoschaMeyer Your opinion and feedback is welcome! I'd also still like to see syntax detection as a plugin. Unless proven to have a poor performance. Because I still like the idea to have an AST based syntax highlighting for certain languages (besides, as already discussed on some other issues, the challenges that are attached with this approached).
@cmyr Could summary :) |
Sublime has a solution to this problem. First, it has a document describing standardized scope names. This lets syntax authors and theme authors coordinate. It also makes the scopes useful for plugins to interpret. And since scopes are actually a stack, things like "expand to scope" can work quite well, it just keeps expanding up through hierarchical constructs (brackets, block, function, module) as you continue pressing the key binding. Similarly, the scope names for marking function definitions are standardized so "Go to function" can work, and there's a config file to add things like module definitions if you want to add things to it. Also you mention a As for if this should be done in plugins instead of the core. I think of scopes more as a coordination mechanism than a piece of functionality. They don't actually do much on their own, I think they're an excellent candidate for inclusion in the core. I don't think they should be used for the front end as well though, it would be sad if every front end had to unnecessarily implement their own theme engine to apply colours. It makes much more sense to do it in the core and send colours/styles in some (perhaps semi-compressed form) to the front end. |
I'm in favor of scope->style resolution being a separate crate which is included in the xi core, for the arguments @trishume has outlined. I'm finding that for any piece of functionality, there are tradeoffs from doing it in the front-end, the core, or plugins. For scope->style resolution in particular, I think the core is the right place. It could be done in front-ends, but then every front-end would need to duplicate the logic. It shouldn't be done in the same plugin as implementing the syntax, because that would make it a lot harder to use multiple syntax highlighting plugins. It could be done in a separate plugin, but it's hard to see the advantage - you need to have a consistent mechanism for selecting a theme, etc., so it's very hard to see what benefit there is to swapping it out. Also, for certain things (like selecting a background color) you really want it to be fast so there isn't any visible flickering. So count me in favor of that approach. |
Okay, I think we're in broad agreement. I'd like to roadmap this, since it is pretty tightly coupled with plugin support. Some Qs:
|
|
Okay, so I guess we're moving on this: trishume/syntect#70 is merged, which means that the highlighting + scopes components of syntect are now available without having to use the parsing module. This necessarily involves some architectural changes. My initial thoughts:
There's a new This gets us most of the way there. Some things I'm not sure about:
|
One thing to note is that Sublime/Textmate/Atom/etc don't actually store one scope per character, but each span actually has a scope stack. There's a couple ways Xi could do this:
Re first question:Sublime solves this by having highlights from plugins be totally separate from syntax highlighting, but I'm not sure if we should take that approach. It avoids conflicts, brokenness and weirdness, but it limits possible plugins like highlighting identifiers in rainbow colours, or based on type. One easy solution is just to join the scope stacks of various plugins in some order, and then theme selector matching precedence logic will determine which wins. Re second queston:Plugins should be able to submit lists of (selector, style) pairs, these are then concatenated in some order based on some kind of plugin precedence, then used with syntect to match against scope stacks. Selector matching precedence logic then solves most interference concerns. |
Quick thoughts. I think It sounds like your question about defining additional scopes boils down to whether themes can be extended through some other mechanism. There are generally lots of scopes that don't get styles. There's more to say but I have limited time right now. |
I talked to @raphlinus about this IRL, and his idea (which I agree with) is to represent this in syntect as One way to do this, and that ties in well with how we are talking about #318 is that scope stack IDs are unique per layer, and are allocated in order so that lookup is in a Then the theme engine would apply This can be made to slow down tremendously for pathological files (e.g 10000 open braces) but since syntax highlighting is a plugin this won't do anything other than make highlighting slow for that file. The representation of pushes pops and scopes that syntect uses handles that case fine, but it's really not necessary. |
Okay, to clarify:
An invariant I am assuming: within a layer, spans will not overlap, e.g. a span represents the complete |
Yup, that's all what I'm thinking. Including spans not overlapping, except perhaps between layers where you'll have to concatenate scope stacks or something before doing theme resolving. |
@trishume: Okay, one thing that's bugging me right now: In the plugin, I'm going to have a |
Yah it is. But I don't think it is that bad:
|
So I modified the Basically the amount of unique scope stacks in Rust and JS files (two syntaxes that use meta scopes for blocks and so generate more unique scope stacks than other syntaxes). The data below was generated with this branch of syntect, look there to see what the numbers mean. But basically sending all the scope stack ops in a binary encoding would take about 1mb for jQuery, but sending the span stack ids and the unique stacks as strings in a fairly compact JSON encoding would take 3.3mb for the stacks and another 1mb for the spans with stack ids. So about 4x as much data. I haven't yet benchmarked the overhead for doing highlighting calculations on unique spans and the overhead of the data structures to encode stacks as unique spans. Not sure if this information changes anything, haven't thought about it enough yet. Edit: one thing to note is that the unique stacks only need to be sent really once per file. The stack repo can probably be kept around and won't need to be added to much on large files, so it's a cost to be paid on first highlight. Another thing I noticed is that this cost could be dramatically reduced because most new stacks in the stack repository will be additions to a previous stack, so there could be a message representation like "add new stack starting with id 2524 and adding
|
This is progress towards xi-editor#284 / xi-editor#319. Instead of explicitly sending style information (e.g. actual colors) plugins now send scope information, which is resolved into colors in core. This is incomplete: importantly it has dropped the ball on the important question of how to define custom styles, and how to merge scope definitions from multiple plugins.
Okay, so there are two major outstanding issues, once #329 lands. Span mergingSpan merging involves coalescing multiple layers' spans into a single tree, resolving conflicts. The algo is fairly simple: working with two layers at a time, you merge each overlapping section of spans into a new span.
or:
My inclination is to add this as a new operation on I'm leaning towards the first approach, combined with making @raphlinus any possible problems with this approach? Style definitionsOne of the major impetuses for this work was to allow the declaration of custom styles. This is the area I've thought about the least, in terms of how style defs should fit into the existing theme system; I'll give this some thought a bit later. |
@trishume okay another quick thought/question about stack merging / conflict resolution. Let's say we're the syntect plugin, in a rust file. For some span, we send Basically I'm wondering if you think that in this case we merge the stacks, or if we resolve to a style for each stack and then merge those? In the first case, is there some conflict-free way of determining the stack-merge order? In the second case, since we couldn't trust the theme to correctly privilege the styling associated with the error, we would have to associate some sort of priority to that style; this would have to be out of band, e.g. sent alongside that scope when that scope was defined by the plugin? (I think). Let me know if this doesn't make any sense. |
There's two avenues. Both of them need each layer to have a "priority" to determine which to favour when there is a conflict.
|
There's two ways I can think of for doing priority:
|
So I’ve been thinking about this again: I think there’s some conflation of things going on here, or at least I’ve been conflating some things. I don’t think that the types of highlighting that is presented by a linter is a style in quite the same way that syntax highlighting is a style. That is: syntax highlighting is a special case of style. Something that comes in from a linter could (and probably would, should) have a presentation style that is distinct from the styles available to syntax highlighting: for instance a colored wavy underline in ST. This is also distinct from (at least some) other forms of annotations; for instance if some identifier has documentation available on hover, we might represent that in a distinct group of spans. Anyway: I’m trying to zoom out a bit and think about exactly what a style is, and exactly that we’re merging. Do we represent a lint as a distinct style, and the frontend is responsible for drawing it alongside syntax highlighting, or do we define a new style for each highlighting+lint combination? Do we allow multiple syntax highlighting plugins to be active in a given buffer? Plugins that provide scope information could be a distinct type, and we could choose to run only one of them using some rule of precedence. In any case, it is unclear to me that the problem of merging styles in the current theme system is a big problem, because most of the conflicting styles provided by various plugins seem (to me) to be styles of separate types. The question is whether and how we want to merge these. |
@cmyr yes this is a real concern, and there are two approaches that I know of in existing editors. Personally I'm fine with either, it's a tradeoff where you can sacrifice some power for the benefit of plugins not interfering with each other as much (which is a real benefit Sublime has over things like Emacs). The Sublime ApproachSyntax highlighting is a privileged type of styling that uses a different mechanism than linters and plugins. Syntax highlighting determines the color, background and font style of the text. Plugins can then use Pros:
Emacs MethodIn Emacs plugins can do any highlighting they want and there's no super-clear distinction between a syntax highlighter and something like a linter. Pros
|
Okay, I've done some more thinking on this, and I have an approach that I'm mostly okay with. This is an amalgam of the two approaches @trishume outlines above. Proposal: two style APIsI propose that there be two distinct APIs through which plugins can modify text styles. One would be the current scope-based system, and would be the general approach for syntax highlighting; the second would use named identifiers, and would be preferred for special cases, and would have access to richer style definitions. Merging styles from both of these sources would be possible. syntect styles & xi stylesThe style information provided by syntect (& supported by We imagine that ultimately xi will support a superset of these, including things like overline, strikethrough, css-style
syntax highlighting stylesPlugins which are providing syntax highlighting will operate as they currently do. This API hasn't been thoroughly described yet, but basically the plugin defines scopes, which it sends to core, and then sends span+scope groups to describe the styling for various text regions. (for more info on scopes see the sublime text docs.) When custom stylesA plugin which wishes to define custom styles skips this intermediate step. Instead it sends an These styles are then assigned to spans through the same RPC as is currently used with scopes. drawbacks
Also (cc @trishume) A limitation of our current implementation is that scopes are defined in a per-plugin/per-buffer basis, so two plugins sending scopes to the same buffer don't know about each other or each other's scopes. This is a structural reality of using an index based API. It's unclear to me whether or not this is a serious limitation or not. but if we were augmenting the theme format...One possibility for this would be to allow theme authors to define certain named colours, e.g. resolutionIn This mechanism gets us more or less where we want to be. If anyone thinks this is a bad idea, let me know, because I'm going to start implementing it. 😏 bonus round: Styles vs Annotationsor: how should a linter actually work? In a lot of this thinking about merging styles, our example has been merging styles from something like a linter with styles from syntax highlighting. A linter is more than just a style; it's an annotation. Does a linting plugin send two different types of spans: styles and annotations? Or does it send a single annotation span that maps to a style and has associated metadata? |
@cmyr that sounds good, the only thing I might add to that proposal is that like Sublime, you can make the custom styles fit with the theme by mandating that the color be specified as a scope. So for example linters typically use the That way the plugin decides style attributes that don't really depend on the theme (is it a border? fill? underline? squiggly underline?) and the theme decides the color. If themes want to specify a certain color (dunno why they would?) we can have a convention for themes like I don't think the separate scope ids per layer is a limitation. It allows the syntax highlighting plugin to not have to wait for the core to tell it an Id for the stack/style it just sent before using it. If it didn't do that the round trip waiting would just kill performance. Sharing between layers of the same buffer wouldn't be much of a win since normally there will only be one syntax highlighting plugin, and if there are multiple they'll probably be using different scopes. Sharing between buffers of the same file type (or all buffers) would save memory, but I'm not sure how to do it without losing the no-round-trip property, and I'm not sure how big a win it would be. Per layer should be fine for the foreseeable future. |
Okay, so then the only funny part (to me) is this: if we use scopes for custom styles, how do we decide when to use the theme's colors and when to use the custom style's? |
@cmyr the custom style's color is based on the theme. Possibly just as the recommended way of setting the color, possibly as the only way to prevent people from making theme-incompatible plugins. So the serialization of a At some point it will probably be a good idea to extend syntect and Xi to be able to add/remove themes in an ordered list of precedence so for example if |
@trishume I guess my main hangup here is that in the general case a scope is resolved to a style, i.e. fg+bg+text_styles, and it feels a bit strange to override this, and resolve scopes in some contexts to just their In any case I'm not really implementing region stuff right now, so I'm happy to punt on some of the details and take some more time to mull it over. I appreciate all of your input, you've obviously thought about this problem a bit. 😎 I will have a PR shortly that finishes off this preliminary style work. At some point this week I'd appreciate if you could get a syntect release out with my last patch, but maybe hold off a few days and make sure I don't have any other needs. |
@cmyr My entire thinking process is to look at what Sublime does and think about why they made those choices, and they are often good reasons. In Sublime's case using the
|
Delay hiding of statusbar to avoid flicker
Current Situation
We have a
set_style
RPC call (which is actually calleddef_style
in the source):This is intended to be used by syntax highlighting, linting and similar plugins, which set color spans for parts of the text. Instead of having to define and therefore repeat the style for each span separately,
set_style
allows to reuse them.When a
Line
is send from the core to the frontend, it could contain anumber[]
array which defines the style spans of this line.To quote the current
update.md
:Shortcomings
When having different plugins that set style spans on their own (1 syntax highlighting, 1 linting, ...), we probably end up with colors that do not match well (i.e. how does the lint plugin know to use a red which matches my dark or light theme).
Possible Solution
As in other editors, styles could be represented by a string (often called a "scope" in other editors). For example Sublime Text defines the following for a minimal scope coverage:
However, to not restrict plugins in their power, we might not predefine/restrict these scopes and let plugins define them theirselves.
So the Syntax Highlighting plugin might define a style for
invalid
, while a Lint plugin re-uses thisinvalid
style definition or defines its own, if a style forinvalid
does not already exist.That is, the
set_style
RPC call would have two different semantics, one that overrides a style, and one which only sets the style if it does not already exist:Discussion
set_style
be to a tab, or to the global session?number[]
would not be possible anymore?The text was updated successfully, but these errors were encountered: