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

Syntax highlighting colours function names differently when starting with capital #311

Closed
DanTup opened this issue May 25, 2017 · 13 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is enhancement
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented May 25, 2017

Not sure how easy this will be to fix, but TypeScript doesn't seem to have this problem. We should be able to identify functions from the parens?

Colours

@DanTup DanTup added is enhancement good first issue :octocat: A relatively straight forward issue that could be done by a new contributor labels May 25, 2017
@Skylled
Copy link
Contributor

Skylled commented May 25, 2017

Isn't this because capitalization implies a Class name instead of a function?

@DanTup
Copy link
Member Author

DanTup commented May 25, 2017

That's the convention; but you don't have to follow it. If we were using the data from the analyzer, I'd expect they would be coloured correctly (based on actual AST, not guesses based on casing) so ideally we should be the same (whether it's reasonable or worthwhile, I don't know.. maybe we should just hope for microsoft/vscode#1967!)

@DanTup DanTup added the in editor Relates to code editing or language features label Jun 24, 2017
@Skylled
Copy link
Contributor

Skylled commented Jul 23, 2017

Looking at this again on a whim, maybe we could assume that any ambiguous capitalized object that is followed by parens but not preceded by new or const is a function.

class IsClass {}

void NotClass(){}

var x = new IsClass();
var y = NotClass();

But that still won't help when assigning a function to a variable.

someObj.callback = NotClass // There's nothing here to detect from.

Additionally, the inverse problem is also true, lowerCamelCase class names are not treated correctly.

class alsoAClass {} // No colorizing
var z = new alsoAClass(); // Colored as a function

Seems like the best solution would be to get direct from the AST, as you said.

@DanTup
Copy link
Member Author

DanTup commented Jul 24, 2017

I think using the parens is probably the best way; seems more reliable than guessing based on case. I don't think Code is going to support dynamic colouring anytime soon :(

I'll see if I can change the regex at some point to handle this better.

@DanTup DanTup removed the good first issue :octocat: A relatively straight forward issue that could be done by a new contributor label Feb 2, 2018
@DanTup DanTup added this to the Backlog milestone Feb 21, 2018
@DanTup DanTup added the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Apr 2, 2018
@weenzeel
Copy link

weenzeel commented Jun 16, 2019

Found another case where upper case is tripping up the syntax highlighting:

class FileMode {
/// The mode for opening a file only for reading.
static const read = const FileMode._internal(0);
@deprecated("Use read instead")
static const READ = read;
...

In this snippet lower case read is scoped as source.dart but upper case READ is scoped as support.class.

@DanTup
Copy link
Member Author

DanTup commented Jun 17, 2019

@weenzeel thanks for the example. It's hard (for me) to fix some of these issues with the textmate grammar. I did hope to try and generate it from the language descriptions (in the hope it'd be accurate, including fixing the comment-highlighting bugs that keep coming up) - see dart-lang/sdk#19298, but so far I haven't found a good way.

@weenzeel
Copy link

@DanTup I opened a syntax issue with Jetbrains and Dart SDK earlier this year. Feedback from Jetbrains was that they depended on the Analysis Server for Dart syntax highlighting. Not sure how that works or viable for VSC as well?

dart-lang/sdk#35679

@DanTup
Copy link
Member Author

DanTup commented Jun 17, 2019

The Dart server provides this info, but VS Code doesn't allow extensions to perform colouring programtically:

microsoft/vscode#585

I have seen a workaround someone made that uses the VS Code decorations API to provide the colours - it's an interesting idea, but I haven't dug into how feasible (or reliably) it would be here.

@lukepighetti
Copy link

lukepighetti commented Dec 11, 2019

tbh if you left it the way it is, i think it's fine. Typescript tooling is unopinionated, but Dart tooling is typically very opinionated. Just my opinion. 😄

@DanTup
Copy link
Member Author

DanTup commented Dec 11, 2019

I would like to improve our syntax highlighting generally - we currently have a TM grammar file that doesn't accurately reflect the language and we often have bugs. Currently this is the only option for VS Code, and I don't really want to do too much more with that file - however it seems there may be some changes incoming (see microsoft/vscode#77140). It's not clear if that will allow us to be completely dynamic (eg. delegate to the server) or it'll just be a different format. I have some concerns that going to the server may be too slow. If it is just a new file format, I'd at least like to try to generate the file from the Dart spec so that it's reliable and easy to update.

@DanTup DanTup added the in lsp/analysis server Something to be fixed in the Dart analysis server label Oct 27, 2020
@DanTup DanTup modified the milestones: Backlog, On Deck Oct 27, 2020
@DanTup
Copy link
Member Author

DanTup commented Oct 27, 2020

This will be handled as part of #2202. It works using the semantic tokens and LSP, but the API hasn't been finalised in LSP so we can't finish/release it yet.

@DanTup DanTup removed the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Nov 25, 2020
@DanTup DanTup modified the milestones: On Deck, v3.18.0 Nov 25, 2020
@DanTup
Copy link
Member Author

DanTup commented Dec 15, 2020

Fixed by dart-lang/sdk@cb2ede5.

In order to get the fix you'll need Dart-Code v3.18.0 (a preview release should be available later today/tomorrow), be using LSP Preview, and have a Dart SDK from after yesterday (the most recent nightly works, and of course the next releases of Dart/Flutter SDKs will include this).

semantic_tokens

@DanTup DanTup closed this as completed Dec 15, 2020
@DanTup
Copy link
Member Author

DanTup commented Dec 22, 2020

Due to some last minute issues, support for LSP 3.16 (which supports semantic tokens) won't be included in v3.18, but hopefully v3.19 next month instead.

@DanTup DanTup modified the milestones: v3.18.0, v3.19.0 Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is enhancement
Projects
None yet
Development

No branches or pull requests

4 participants