search

microsoft / vscode-html-languageservice

Language services for HTML
MIT License
220 stars 109 forks source link

Update js-beautify v1.15.1 to add new angular templating option #181

Closed simon-knu closed 6 months ago

simon-knu commented 6 months ago

This pull request addresses the open issue #177.

As I don't often submit pull requests, I would appreciate any feedback on the implementation and/or description.

simon-knu commented 6 months ago

@microsoft-github-policy-service agree company="Corellia SRL"

NesTeRDGIT commented 6 months ago

It seems to me that this PR will not work Because: 1) getTemplatingFormatOption always returns ['auto'] or ['none']. A should return parameters set by the user, for example ['angular']. 2) HTMLFormatConfiguration.templating type should be - ('auto' | 'none' | 'angular' | 'django' | 'erb' | 'handlebars' | 'php' | 'smarty')[]

getTemplatingFormatOption function should return HTMLFormatConfiguration.templating HTMLFormatConfiguration.templating itself must be filled in by the user in the VSCode parameters. I don't know how to do this...

I think the VSCode repository needs to be updated: https://github.com/microsoft/vscode/blob/main/extensions/html-language-features/package.json

simon-knu commented 6 months ago

@NesTeRDGIT You were right

I pushed a change, thank you for your feedback

IMO, the best thing to do is to keep the "boolean" property with the possibility to add the js-beautify configuration Like that, we don't break existing configs

This means that we need to do something like that (see picture) in the vscode repository to be able to configure a boolean or an array of values

I just need to confirm that this kind of config is possible, but it would hope it is :)

image

What do you think ?

NesTeRDGIT commented 6 months ago

I am not a VSCode developer So I can't confirm this. Here we need the opinion of VSCode developers

But I think VSCode will not accept such configuration because it is js-beautify private settings. These are not common options for all HTML extensions.

I was suggested to the js-beautify developers to develop an extension for VSCode, for example like: https://github.com/HookyQR/VSCodeBeautify @bitwiseman

bitwiseman commented 6 months ago

@NesTeRDGIT

I am not a VSCode developer So I can't confirm this. Here we need the opinion of VSCode developers

But I think VSCode will not accept such configuration because it is js-beautify private settings. These are not common options for all HTML extensions.

I was suggested to the js-beautify developers to develop an extension for VSCode, for example like: https://github.com/HookyQR/VSCodeBeautify @bitwiseman

The problem is that "js-beautify developers" is mostly me. I don't have the time to be primary maintainer on a VSCode plugin. If you are interested in forking the HookyQR beautifier plugin into https://github.com/beautifier and doing the work to spin up that project, please open an issue over in js-beautify and let's chat.

aeschli commented 6 months ago

The HTML language service formatter and (VS Code) currently has an on/off setting for template support. This mapped to auto or none. This results in simple user experience and has worked well so far.

I wonder, do you know the reason why angular nor enabled by default in auto ?

aeschli commented 6 months ago

Reading more about it, I see that the angular support got turned off due to performance reasons: https://github.com/beautifier/js-beautify/issues/2246

The thing is, the HTML language server is intended for HTML as understood by browser. Templating languages are extensions of HTML and, if they extend the syntax, not HTML. The HTML extension does it's best to not been thrown off by the extended syntax, and I know a lot of users still use it also for template languages, but the recommendation is to instead use an extension that is for that template language, e.g. an Angular extension

We can add the new options to vscode-html-languageservice, but I'm not in favor of complicating the settings of the HTML formatter in VS Code with a list of templating languages. That would give the impression that the HTML language extension is also for these languages, which it isn't.

simon-knu commented 6 months ago

Thank you for the feedback @aeschli

I understand your point, but since the options are there, it feels a little bid odd (for me) to not allow users of VS Code to use it I mean, with the configuration proposed here, it won't change anything for the "basic user" When I say "basic user, I just mean user that doesn't want to configure more explicitly the formatting, don't get me wrong :)

That would give the impression that the HTML language extension is also for these languages, which it isn't.

It was maybe not intended at first, but with the dependency on js-beautify, now the HTML language extension is linked to the languages supported by the lib.