angular / vscode-ng-language-service

Angular extension for Visual Studio Code
MIT License
775 stars 116 forks source link

Syntax Highlighting for Angular Template Syntax #483

Closed ayazhafiz closed 4 years ago

ayazhafiz commented 4 years ago

Is your feature request related to a problem? Please describe.

As an extension of the new Textmate grammar provided by this extension, we should provide a grammar for the Angular template syntax.

Additional context

See https://github.com/angular/vscode-ng-language-service/issues/395 for the inline template syntax highlighting discussion.

Running list of features:

jpike88 commented 4 years ago

@kyliau some useful CSS examples to cover:

in the one below, functions are just orange, they should stand out 68101848-52761a80-ff02-11e9-8c07-681380946ce0

these two should be obvious enough 68099924-d4ad1180-fef7-11e9-8b4c-0a36e983da0b Screen Shot 2019-12-17 at 10 43 49 am

ayazhafiz commented 4 years ago

@jpike88 I can't seem to reproduce your CSS examples in a template or CSS file. Are they located elsewhere?

jpike88 commented 4 years ago

These are inline templates, in a @component definition

ayazhafiz commented 4 years ago

@jpike88 do you have the angular2-inline extension installed? I see what you're getting with the CSS styles, but only when that extension is enabled. If you turn off that extension but keep the language service extension enabled, you should see correct CSS highlighting in an inline template (styles in a styles property on a Component won't be, though).

jpike88 commented 4 years ago

@ayazhafiz you're right, the inline extension did modify colours. but its worse in some ways with just the angular extension. new screenshots:

Screen Shot 2019-12-18 at 12 14 53 pm

Screen Shot 2019-12-18 at 12 15 21 pm Screen Shot 2019-12-18 at 12 15 43 pm

ayazhafiz commented 4 years ago

@jpike88 can you take a look at the screenshots at #512 and let me know if that’s along the lines of what would be better?

jpike88 commented 4 years ago

yeah way better, nice. just need more advanced solution for the inline html template now

ayazhafiz commented 4 years ago

Great! Support for the Angular template syntax will be coming soon.

jpike88 commented 4 years ago

found weird edge case hopefully that PR solves it too Screen Shot 2019-12-18 at 1 38 52 pm

a-gerescher commented 4 years ago

If i add an opening bracket in css like this .style{ the whole component syntax highlight is broken.

ayazhafiz commented 4 years ago

If i add an opening bracket in css like this .style{ the whole component syntax highlight is broken.

This likely will not be fixed unless we move to a more advanced syntax highlighting solution than the TextMate grammar one used by VSCode. This also happens in the native HTML syntax highlighting. It should be at least partly mitigated by auto-closing brackets.

ghaschel commented 4 years ago

This extension is breaking my HTML highlighting, as it is mainly a language service, i think its highlighting should be optional.

Before activating this plugin: Screen Shot 2019-12-27 at 12 16 09

After activating: Screen Shot 2019-12-27 at 12 16 31

It breaks my highlighting and every thing inside {{}}

Don't get me wrong. I love angular-language-service, but i like my highlighting better

ayazhafiz commented 4 years ago

Great point. We should hide behind the highlighting behind some enable/disable flag. This might not be possible, in which case maybe the syntaxes should be another extension entirely.

Relevant: https://github.com/Microsoft/vscode/issues/10565, https://github.com/Microsoft/vscode/issues/26707. Looks like turning off non-programmatic extension contributions may not currently be possible.

ghaschel commented 4 years ago

I did manage to find a solution to this problem. Changing my scopeName from text.html.angular to template.ng.

As textmate grammar is already something tricky to train/study, I think it would be very convenient for developers to have language service and highlighting separately.

ayazhafiz commented 4 years ago

Yeah, we shouldn't have to force anyone to change their grammar to get around ours.

jpike88 commented 4 years ago

P.S can't wait for inline html/css auto-formatting after this one, it will be kickass!

ayazhafiz commented 4 years ago

@jpike88 more features are being added, see the PRs listed in the first comment - in fact, we already have support for JS syntax highlighting inside interpolations :-)

The question is whether this extension’s scope is an Angular language server or an Angular IDE. I think it’s an Angular language server, in which case we should offer a way to opt out the non-critical functionality of the extension (notably, the syntax highlighting), even if our grammars are “correct”.

ghaschel commented 4 years ago
  • @ghaschel The only different in that last one I see is the enhanced highlighting of double brackets, am I missing anything else there?

@jpike88 in that screenshot is only the brackets. But i ended up losing highlithing for pipes, elvis operator and a few other small things. But i fixed it. Matched this project grammar main scope name.

ref- is an alternate syntax for template variable declaration. Highlighted that way to differ from normal html attributes

dannymcgee commented 4 years ago

Hey @ayazhafiz and co., I'm the author of a standalone grammar extension that adds some pretty detailed grammar scopes for Angular templates. I've been using it as a full-time Angular engineer since August and it's pretty robust. Unfortunately, I noticed today that many aspects of my extension are no longer working, and tracked the problem down to this extension, which I assume is overriding my own grammar.

I think it's a great idea to have this feature as part of an official Angular extension, but I do miss the small details of mine which are now missing. My extension is MIT licensed, so you're more than welcome to borrow from it. Alternatively, I could submit a PR (or several) that adds the missing details to this extension. Which would be the best option? I'm not seeing any contribution guidelines in this repo so I just want to make sure I'm going about things the right way before I expend too much effort on this. Thanks!

ayazhafiz commented 4 years ago

re @dannymcgee Hi Danny, we’d definitely love your contribution! I’ve been working on the grammars, but they’ve been coming along slowly and are not yet the most robust. Please feel free to send PRs for any features and fixes you’d like to see.

All the info about the grammars provided by this repository should be in the syntaxes/ folder. Please reach out if there’s any help we can provide.


From: Danny McGee notifications@github.com Sent: Friday, January 10, 2020 5:13:51 PM To: angular/vscode-ng-language-service vscode-ng-language-service@noreply.github.com Cc: hafiz ayaz.hafiz.1@gmail.com; Mention mention@noreply.github.com Subject: Re: [angular/vscode-ng-language-service] Syntax Highlighting for Angular Template Syntax (#483)

Hey @ayazhafizhttps://github.com/ayazhafiz and co., I'm the author of a standalone grammar extensionhttps://github.com/dannymcgee/vscode-ng-html that adds some pretty detailed grammar scopes for Angular templates. I've been using it as a full-time Angular engineer since August and it's pretty robust. Unfortunately, I noticed today that many aspects of my extension are no longer working, and tracked the problem down to this extension, which I assume is overriding my own grammar.

I think it's a great idea to have this feature as part of an official Angular extension, but I do miss the small details of mine which are now missing. My extension is MIT licensed, so you're more than welcome to borrow from it. Alternatively, I could submit a PR (or several) that adds the missing details to this extension. Which would be the best option? I'm not seeing any contribution guidelines in this repo so I just want to make sure I'm going about things the right way before I expend too much effort on this. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/angular/vscode-ng-language-service/issues/483?email_source=notifications&email_token=AE6GL6QUWPRAU34XAWRLAJTQ5EMM7A5CNFSM4JXFOFWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIVU5XA#issuecomment-573263580, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AE6GL6SX3G52KF2TZ6DGNR3Q5EMM7ANCNFSM4JXFOFWA.

dannymcgee commented 4 years ago

Hey @ayazhafiz, I'm developing on Windows 10 and I'm getting this error when I try to run yarn test:syntaxes -u:

Uncaught exception: Error: spawn yarn ENOENT

I tried it with git bash and PowerShell, same result in both cases. I installed the repo dependencies with yarn install, do I need to do anything else to configure my dev environment? Thanks for your help!

ayazhafiz commented 4 years ago

Does this happen on master? I think running yarn should be enough.

dannymcgee commented 4 years ago

I managed to get it working by using Windows Subsystem for Linux :)

kyliau commented 4 years ago

Hi @dannymcgee! Thank you for reaching out, and I apologize for breaking your extension with the recent changes that we made to the Angular language extension. @ayazhafiz and I had a brief discussion about this, and it is certainly not our intention to break other extensions that provide similar Angular template grammars. We are still exploring a solution that would provide both good user experience and compatibility with third party extensions. One suggestions that Ayaz brought up is bundling the Angular extension as an extension pack, and and it will consists of two components - one would provide language service, and the other provide textmate grammar. The other solution would be to consolidate the grammars into the first-party extension. There are pros and cons to both, and we would love to have your input on this. Thank you very much!

dannymcgee commented 4 years ago

Hi @dannymcgee! Thank you for reaching out, and I apologize for breaking your extension with the recent changes that we made to the Angular language extension. @ayazhafiz and I had a brief discussion about this, and it is certainly not our intention to break other extensions that provide similar Angular template grammars. We are still exploring a solution that would provide both good user experience and compatibility with third party extensions. One suggestions that Ayaz brought up is bundling the Angular extension as an extension pack, and and it will consists of two components - one would provide language service, and the other provide textmate grammar. The other solution would be to consolidate the grammars into the first-party extension. There are pros and cons to both, and we would love to have your input on this. Thank you very much!

Hey @kyliau, sorry I'm just seeing this! It's really no problem at all. I think this extension is the natural place to include the grammar definitions — it's pretty typical for 1st- and 2nd-party extensions that provide language support (e.g., the C# and C++ extensions). In fact, when I first started getting into Angular development a few years ago I installed this extension and was confused and mildly annoyed by the fact that it didn't provide syntax highlighting. :P

The main advantage of providing it here is that it gets far wider use — this extension has 1.2 million active installs, while mine has about 2,700, and another one has about 21,000. You have to really go out of your way and know what to search for to find either and I think for most people it's just not worth the effort (or they don't even think of it).

The issue of overriding 3rd-party extensions that provide the same functionality is potentially problematic, but with grammars, I think if the scopes provided are granular enough there's not much difference in the end result (though it will likely require themes that supported a 3rd-party grammar previously to update the scope names they're targeting).

I think the reason this extension (partially) overrides mine and @ghaschel's is because ours are defined as outright replacements for the full HTML grammar, while this one is just injecting new patterns into the existing grammar. (When I noticed the problem the other day, it was only the property binding attribute names that were off — the punctuation tokens were missing — while the event binding, structural directives, etc. were still being handled by my extension.) On the one hand, this is really the correct way to do it since you're not duplicating a ton of work or forking from a version that could be updated in the future. But on the other hand, I'm really not sure if it's possible to implement it this way and also avoid overriding or conflicting with other extensions. If you were to make it optional, I think I would prefer a checkbox in the VS Code settings menu over an additional extension, but I wouldn't know how to implement that either. :)

All things considered, I think having one extension that provides language support, including syntax highlighting out of the box, is really the ideal situation — as long as the grammar definitions are granular and robust enough that theme authors can really go nuts and do whatever they want with it. If it had been included back when I first started developing with Angular, I could have saved a whole lot of time hacking together my own solution. I would like to hear @ghaschel's thoughts on this also though — I think his is the only other extension on the marketplace that would be affected by the changes here.

jpike88 commented 4 years ago

I think making this extension as all-encompassing as possible with the ability to turn on-off certain features is the way forward. While we should accommodate the few who want to turn off some things so another extension can override, aiming for a homogenous development environment using the 'it just works' principle is a good approach for a framework like this, allowing people to just kick things off as easily as possible

ayazhafiz commented 4 years ago

Do you all know of popular themes for Angular relying on textmate grammar scopes?

ghaschel commented 4 years ago

@ayazhafiz I, myself, like my highlightings to be contextually colorful and distinctive, and this is where I aim with my package. But of course, this is just my taste, and it is something very personal. There are a few ones I suggest in my my extension: dark-plus-syntax (it is basically the default dark theme with extra missing scopes), cobalt2, Dracula, and an old hope

@dannymcgee Firstly, thanks for mentioning my extension :smiley: I have managed to override the problem caused by this extension by applying my grammars to two scopes (HTML and ng.template - the one this extension and angular2inline uses as well - I think this should be pointed out). Had to generate 2 json files (wasn't a big deal, I just had to build twice, and set the the first level scopeName after building to both final files). And it works just fine now.

@jpike88 I agree. I took all-encompassing as my norm when developing vscode-angular-html, covering Angular and alternate syntaxes, angular material and the HTML5 specification itself (invalid attributes, deprecated tags, dom events, etc...). ["While we should accommodate the few who want to turn off some things..."] But that would imply in regenerating the json file, wouldn't it? (I am using json5 as my src, so that I can have a better organization and code overview, I would like to encourage you guys to take a look and let me know what you think of it). Now that vscode doesn't require you to reload after applying a syntax highlighting, I think we could make this work seamlessly.

I have had feedbacks from people saying that for them it is too colorful, and that they would love to have a way to customize colors or to choose from a colorful one to a less colorful one. Maybe having it available for people to have both mine and @dannymcgee s to choose from as options?

dannymcgee commented 4 years ago

I have had feedbacks from people saying that for them it is too colorful, and that they would love to have a way to customize colors or to choose from a colorful one to a less colorful one.

This is the sort of thing that I really feel should be the responsibility of theme authors. There are lots of very minimal themes out there that only use one or two colors in various shades or that decline to color certain types of tokens at all so they remain the default foreground color. As long as the scopes provided by the grammar are detailed enough, theme authors have a whole lot of flexibility to decide exactly how to apply (or not apply) colors to the code. What has always annoyed me about grammars (as a theme author) is when they simply don't offer that granularity, so if I want to color a certain token a certain way in a certain context, I have to go and screw with the grammar to make it possible.

A shorter way to say all that is that grammar scopes are a lot like CSS classes, with themes serving as the stylesheet. More classes means more flexibility for the folks writing the stylesheets.

EDIT: And it is actually possible for end users to customize their theme via the settings.json. The Inspect TM Scopes command will reveal all grammar scopes applied to the text under your cursor and users can add theme override rules to their settings to apply styles to those scopes. So for example, if there was anyone out there who really didn't want any special highlighting for their Angular templates, they could theoretically just add a rule to their settings targeting template.ng source.js and set it all to the string color.

dannymcgee commented 4 years ago

Do you all know of popular themes for Angular relying on textmate grammar scopes?

I would be the wrong person to ask tbh — I've only ever used my own theme and I only even published it to streamline the process of syncing it between my personal machines and my work laptop. :P

dannymcgee commented 4 years ago

(I am using json5 as my src, so that I can have a better organization and code overview, I would like to encourage you guys to take a look and let me know what you think of it).

@ghaschel Sorry, I forgot to respond to this the other night — json5 looks really great for file organization, but I actually really like yaml for not needing to wrap values in quotes (and so not having to double escape everything in the regex patterns). You can also split more complicated patterns onto multiple lines which makes them easier to read. I wonder if there's any way to combine the benefits of both? For my theme I'm actually using plain JavaScript objects so I can use variables, color functions, etc., in addition to organizing my source code and building multiple versions, but I think JSON.stringify mangles regex (or at least doesn't escape it properly).

ayazhafiz commented 4 years ago

Closing, as the template syntax grammar has been completed aside from a custom template expression grammar (#571).

If you are experiencing bugs with the existing grammar or would like another feature, please open a new issue.

angular-automatic-lock-bot[bot] commented 4 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.