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

[HTML] -> Extend the "templating" option to allow an array of options. #208316

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

simon-knu
Copy link

@simon-knu simon-knu commented Mar 21, 2024

This pull request is tightly coupled to PR #181, where we need to introduce a new "templating" configuration.
It's needed to resolve the bug describe in this issue

We need to have the possibility to accept not only boolean, but also arrays of determined options for the "templating" configuration.
To not break existing configuration, I propose to allow both scenario (and keep the legacy boolean option).

This option is needed for the new control-flow syntax released in Angular 17. Thank's to that, we will be able to format ours documents properly with the "html.format.templating": ["angular"]" configuration. At the moment, the format of documents in Angular 17 is broken...

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

@simon-knu simon-knu changed the title feat: extend the "templating" option to allow an array of options. [HTML] -> Extend the "templating" option to allow an array of options. Mar 21, 2024
@andreamah andreamah assigned aeschli and unassigned andreamah Mar 22, 2024
@simon-knu
Copy link
Author

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

@hugoterelle
Copy link

Same problem here, our Angular 17 projects are broken.

@XhmikosR
Copy link

Could someone have a look at this PR please? It's been months since Angular 17 was released and VS Code's built-in HTML highlighting doesn't support the new syntax. Thanks!

@simon-knu
Copy link
Author

@aeschli Can you give some feedback about this PR please ? (or review it ? 😄)

Even if it was not intended to support differents languages at first, it is possible.
It's seems odd to not allow VsCode user's to specify some options supported by the Html Language Service

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2024

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.

For developing Angular, don't you use a Angular extension?

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2024

Is there away to quickly detect that a HTML file uses Angular syntax? We could then, behind the scenes, add the angular item when we use the HTML formatter.

@XhmikosR
Copy link

XhmikosR commented Apr 4, 2024

Well, yeah, but the only plugin that supports the new syntax is prettier, which I personally dislike.

All these years VS Code's built-in formatting does the job quite fine and it'd be a degradation IMHO having to switch to prettier.

If you meant another plugin, there's the Angular Language Service plugin, which I'm not sure if it could/should provide support for the new control flow syntax

@NesTeRDGIT
Copy link

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.

For developing Angular, don't you use a Angular extension?

May be...
Html formatter setting in vscode include abstacts temlating as string array. And any formatter can use this information.

@simon-knu
Copy link
Author

#Is there away to quickly detect that a HTML file uses Angular syntax? We could then, behind the scenes, add the angular item when we use the HTML formatter.

@aeschli I don't think so, some HTLM files in Angular can be very basic without any Angular syntax...

Also @bitwiseman said here that it could cause issue with non-angular users if wrongly detected

That's why I proposed this PR, because the only way to activate the correct formatting is by specify the "angular" option in the Html Language Service

TBH I just want to know if we will have a solution (with this PR or with another one) really soon because it's just not maintainable atm
If not, we will switch to something else, but it's not what my team want

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2024

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2024

The proper thing if if the angular extension provides a formatter for Angular HTML files. That can be based on jsbeautify.

@Tortila90
Copy link

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

@aeschli But it's already supported under the hood. This option is just for additional tuning for those who know what it's for. Proper words can be added to the description so that no one gets the wrong impression.

The proper thing if if the angular extension provides a formatter for Angular HTML files.

Extensions tend to be abandoned after a while. And this particular one is just for a feature that is one step away from being natively supported.

@Tortila90
Copy link

Also @bitwiseman said here that it could cause issue with non-angular users if wrongly detected

@simon-knu This may no longer be the case. I've asked the person who encountered the performance issue and he can't reproduce it, so we may be able to detect angular by default again.

@wedgebert
Copy link

Is this still being actively looked at? As our teams add more of the new syntax, as others have said, it's becoming unreadable without proper formatting. And it's kind of crazy to expect all the developers to install a new extension that is just "default behavior with two parameters exposed"

If the issue is detecting Angular vs non-Angular html files, is that really a problem? I know we'd be fine using the same formatting settings for all files based on a workplace setting or something. No need to say foo.html is angular, but its sibling bar.html is not.

@Harpush
Copy link

Harpush commented Sep 10, 2024

Any news concerning this?

@wedgebert
Copy link

Any news concerning this?

As far as I can tell, it's still an issue. But I did find a workaround. While VSCode's GUI doesn't support the setting, it will take the correct setting in your settings or workspace file.

I added this last tine to my project.code-workspace file and while VSCode shows a warning while the file is open, the built-in formatter will correctly format the new control flow

{
	"folders": ...,
	"settings": {
		"typescript.tsdk": "node_modules\\typescript\\lib",
		"html.format.wrapAttributes": "force-aligned",
		"html.format.templating": ["angular"],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.