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

Turn new angular templating off by default in html #2247

Merged
merged 2 commits into from Feb 16, 2024

Conversation

bitwiseman
Copy link
Member

Description

  • Source branch in your fork has meaningful name (not main)
    The new angular templating caused unintended side-effects when turned on by default. This changes it by off by default.

Fixes #2246

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

  • JavaScript implementation
  • Python implementation (NA if HTML beautifier)
  • Added Tests to data file(s)
  • Added command-line option(s) (NA if
  • README.md documents new feature/option(s)

with self.assertRaisesRegexp(
ValueError, "Selection list cannot" + " be empty."
):
with self.assertRaisesRegex(ValueError, "Selection list cannot" + " be empty."):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are aliases that were retained for backward compat to python 2.x apparently. In typical python style, they just suddenly stopped working on my system. All versions of 3 support the modern names.

@@ -4238,6 +4238,43 @@ exports.test_data = {
' </div>',
'}'
]
}, {
comment: 'CSS @media should remain unchanged',
// This behavior is currently incorrect. This codifies the way it fails.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why #2219 should use a specific list of at-strings. There's just too many ways to break things otherwise.

@bitwiseman bitwiseman changed the title Turn off new angular templating by default in html Turn new angular templating off by default in html Feb 16, 2024
@bitwiseman bitwiseman added this to the v1.15.1 milestone Feb 16, 2024
@bitwiseman bitwiseman merged commit 5150015 into beautifier:main Feb 16, 2024
12 checks passed
@bitwiseman bitwiseman deleted the feat/angular-off-default branch February 16, 2024 16:11
@liesahead
Copy link

@bitwiseman , is it possible to enable this setting currently in VSCode via htmlFormatter options? Because currently VSCode setting accepts a boolean and doesn't list angular as one of the items (https://code.visualstudio.com/docs/languages/html#_formatting).

@bitwiseman
Copy link
Member Author

@liesahead
HTML formatting in vscode is controlled by https://github.com/microsoft/vscode-html-languageservice . That will need to be updated to pull in the new version.

https://github.com/microsoft/vscode-html-languageservice/blob/81cf18fc8aca3ac4ea1e5a99864731a3757260be/package.json#L24

It looks like the settings will need to be updated to allow supplying a list of templating formatters and add angular as an option:

https://github.com/microsoft/vscode-html-languageservice/blob/81cf18fc8aca3ac4ea1e5a99864731a3757260be/src/htmlLanguageTypes.ts#L31-L48

https://github.com/microsoft/vscode-html-languageservice/blob/81cf18fc8aca3ac4ea1e5a99864731a3757260be/src/beautify/beautify-html.d.ts#L119

I've opened microsoft/vscode-html-languageservice#177 requesting update.

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

Successfully merging this pull request may close these issues.

Perf regression in latest release (1.15.0)
2 participants