Skip to content
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

Proposal for (Semantic) Coloring (see #18) #124

Closed
wants to merge 4 commits into from

Conversation

svenefftinge
Copy link
Contributor

Here's a draft for the semantic coloring extension. It's a server side notification similar to publishDiagnostics, which leaves it up to the language server to decide when and if to update the coloring.

We avoided using the term 'highlighting' because it is already used differently. The ids are an initial take, modelled after what we found in monarch with some additional modifiers.

This was referenced Nov 15, 2016
@smarr
Copy link

smarr commented Nov 15, 2016

This would be a great feature to have!

I would expect the ids to be the part that requires most discussion.
Should the ids be explained? This part is also highly language specific, so, I suppose it should cover a combined set of elements for various languages and still be somewhat manageable.

One thing I would like to be able to do is to distinguish field accesses from method invocations based on data from the server side, because in my language it is a unified syntax and coloring could provide a real benefit to code readability.

Not sure why you chose to model different modifiers separately. Is this intentional?

@svenefftinge
Copy link
Contributor Author

One thing I would like to be able to do is to distinguish field accesses from method invocations based on data from the server side, because in my language it is a unified syntax and coloring could provide a real benefit to code readability.

Do you mean something like 'read', 'write', 'declaration'? Would make sense indeed.

Not sure why you chose to model different modifiers separately. Is this intentional?

The idea was to allow combinations of orthogonal things, like "method & static & protected", or "field & private".

Or do you mean the 'modifier_' prefix? That is just because I thought it would be better explain the intended meaning, not sure if that really helps, though.

@smarr
Copy link

smarr commented Nov 15, 2016

Or do you mean the 'modifier_' prefix? That is just because I thought it would be better explain the intended meaning, not sure if that really helps, though.

I was wondering why you have Modifier_* instead of just modifier. The ones in your list are also very language specific. So, perhaps it would be better to have modifier and then in addition visibility (for public, ...), storage (for static), changeability (final, readonly, ...)?

Perhaps the notion of ids is also more a notion of "tags"? I assume here that one can combine tags, while ids are not usually combinable. But that's more about terminology.

@dbaeumer
Copy link
Member

@svenefftinge thanks for the PR. We discussed semantic coloring in the VS Code team as well and we came to the conclusion that, to make it work nicely, it needs to support some delta encoding. So when a files open all sematic colors are computed. When the file is change only delta are computed and sent back to the client. Have you thought about how this could work ?

@dbaeumer dbaeumer modified the milestones: Backlog, 3.0 Nov 21, 2016
@ghost
Copy link

ghost commented Nov 21, 2016

I think it would be great to have some sort of more official support for compiler-based colorization in the language server protocol.

The following is specific to vscode, but I have a prototype implementation of compiler-based highlighting for the agda language at vscode-agda. It uses custom lsp requests and decorations on the client side. Agda allows very flexible syntax (operators can essentially modify the parsing rules) so compiler-based colorization is the only way to provide accurate highlighting. Here is an example of what it can produce (all highlighting is computed by the server):

agda-highlighting

One issue I have run into several times is the need to be able to recover from the client the live decoration positions in a dirty buffer. Sometimes this is needed to work around limitations with the decoration approach (e.g., highlighting must be saved/refreshed on visibility changes) but sometimes this is needed in order to compute new colorization data depending on certain actions in a particular editing context. This is even more important if the server does not have good support for incremental document synchronization.

For vscode, there is currently there is no way to do this through the API so one would have to maintain their own position deltas for the decorations as edits are made. This has negative implications for performance and maintainability.

Being able to send colorization deltas as @dbaeumer suggests would be ideal but I think that approach should be optional, similar to incremental vs full sync for edits. In my prototype, for instance, I am not using a delta encoding because agda does not provide good support for that and it would be much more work and ultimately slower to try and compute deltas on the server.

As for the ColoringStyle ids, I am not sure how useful it is to include them in the protocol. Only a few of the proposed ones would even fit my use case. Here is a list of the ones I use which are determined by the compiler.

@svenefftinge
Copy link
Contributor Author

Yes, incremental update seem to be a needed optimization for large files, but it for sure complicates things which is why I would prefer that to be optional.

How about an optional property invalidationRanges : Range[], that tells the editor which coloring positions should be removed before applying the new ones? If it is undefined we treat it as a full replacement.

We will need to assume that the server and the client move positions according to changes.
Otherwise we would always need to update all ranges after a certain edit, reducing the possible optimization significantly.

@paulotorrens
Copy link

paulotorrens commented Jan 23, 2017

@svenefftinge, I believe that the protocol should be able do handle more exotic languages. The ids in your proposal seem perfectly fit for an object oriented language, but may not have enough options for some languages. E.g., heredoc comments in Perl or Ruby (the later having "end of file data", for which a language server writer might want to give special attention). Maybe J programmers would like a special coloring for hooks and forks.

One cool feature, for example, would be level highlighting, as in:

Ideally, instead of ids, maybe it should accept strings instead of predefined numbers, which could be handled by the client from a map of known names (e.g., "keyword"), or even be handled by the client as CSS classes (as I'm working on a prototype of mine).

What do you think?

@dbaeumer dbaeumer added the feature-request Request for new features or functionality label Apr 12, 2017
@MarkZ3
Copy link

MarkZ3 commented Jun 4, 2017

Any news about this? I'm interested in implementing this in a server soon :)

@haudan
Copy link

haudan commented Jun 7, 2017

Instead of multiple ColoringStyle ids, why not simply send a TextMate identifier as a string? The client side could very easily use such identifiers to drive it's colorization, assuming it's based on a TextMate compatible highlighter.

This would allow for a very broad compability with most languages and color themes (see the TextMate naming conventions).

Basically, instead of ids: int[] => id: String.

ColoringStyle.Keyword => "keyword.baan.ref".


Let's say I'm writing a server for the acient language Baan. The server could transmit identifiers such as keyword.4gl.main.table.section.baan. That identifier is very specific to the language Baan, but because of the nature of TextMate grammars, 99% of all existing .tmThemes would still work properly since they would match against the keyword… part of the identifier.


I don't think things like warnings, errors, etc. should be part of the highlighting response. Isn't this done through other RPC calls anyway?

Edit 1: Shortened and cleared up my comment.
Edit 2: Added naming conventions link.

@haudan
Copy link

haudan commented Jul 4, 2017

Does anyone have any comments on my suggestion?

@Xanewok
Copy link
Contributor

Xanewok commented Jul 10, 2017

Just to chime in about possible usage, it seems that indicating conditional compilation would benefit greatly from this (e.g. C/C++ #if 0/#ifdef, Rust #[cfg(...)], tracked over at rust-lang/rls#384).

@haudan
Copy link

haudan commented Jul 13, 2017

@Xanewok indeed it would! Making code between #if WEATHER_IS_NICE_TODAY = 1 and #endif appear grayed out, would be very useful. Same goes for Rust and languages with similar constructs.

@haudan
Copy link

haudan commented Jul 13, 2017

Addition to my TextMate proposal: The TextMate naming conventions page lists all the de facto standard "identifiers" used by most .tmTheme and .tmLanguage files.

@svenefftinge
Copy link
Contributor Author

Just to chime in about possible usage, it seems that indicating conditional compilation would benefit greatly from this (e.g. C/C++ #if 0/#ifdef, Rust #[cfg(...)], tracked over at rust-lang/rls#384).

Couldn't that be entirely handled by the language server?

+1 for reusing the textmate identifiers.

@Xanewok
Copy link
Contributor

Xanewok commented Jul 14, 2017

Couldn't that be entirely handled by the language server?

It could and it should but from what I understand being able to display differently, e.g. grey out, code that won't be compiled needs the additional proposed notification from the server to the client, no?

@svenefftinge
Copy link
Contributor Author

Yes, sounds right. I think the feature is a bit too specific to a certain kind of language to be added to the protocol, though.

@microsoft microsoft deleted a comment from msftclas Sep 27, 2017
@deathaxe
Copy link

deathaxe commented Oct 2, 2017

Why not use scoping instead of coloring? This is what Sublime Text uses. It creates scopes for source tokens, which are used by color schemes in the second place to assign a color to.

@jpike88
Copy link

jpike88 commented May 11, 2018

I’m guessing this is dead... anyone got any updates?

@haudan
Copy link

haudan commented May 11, 2018

I'm afraid this isn't going anywhere since this PR doesn't follow the contribution guide.

We need to define an protocol extension as described and also provide a reference implementation. I'd love to do this myself, but I no good with TypeScript nor node.

@svenefftinge
Copy link
Contributor Author

I am working on an updated proposal, based on the feedback and following the contribution guide.

@Yurihaia
Copy link

I thing it would be more natural to send a string array of scopes instead of an int array seeing as many text editors use a scope format (Atom, VSCode, TextMate, etc)

@svenefftinge
Copy link
Contributor Author

I thing it would be more natural to send a string array of scopes instead of an int array seeing as many text editors use a scope format (Atom, VSCode, TextMate, etc)

Yes, agreed. We are working on a new proposal, that will use Textmate scopes.

@svenefftinge
Copy link
Contributor Author

Closing in favor of microsoft/vscode-languageserver-node#367

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet