Fixed #2219 - formatting of new Angular control flow syntax #2221

gergely-gyorgy-both · 2023-11-11T23:41:11Z

Description

Source branch in your fork has meaningful name (not main)

Fixes Issue: #2219

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)

gergely-gyorgy-both · 2023-11-11T23:56:26Z

~~Hi js-beautify Team,~~
~~I started fixing the bug mentioned above, but 3 tests are failing and as I couldn't proceed further, I'd like to have some advice.~~

Previously, when the algorithm found the {pre{{field1}} {{field2}} {{field3}}post token, it was treated as a word. Because of the new Angular syntax, the single curly brace in my implementation is treated as a new type of token, BRACE_OPEN. Somehow, I'd have to find the curly brace only in case of control flow syntax (e.g. @if(condition) {), but not in other cases (so the {pre... token should remain as a word). I can only make this work if there is already a space before the { character (but it cannot handle cases where there is not (e.g. @if(condition){), or I can handle both with-space and without-space versions (this is the version commited here), but then it makes 3 handlebarsjs test cases fail.

gergely-gyorgy-both · 2023-11-12T11:28:19Z

Hi again,
I've come to a better solution which only affects the new control flow syntax and doesn't affect the other cases. I'll commit it today after I've cleaned up the code.

gergely-gyorgy-both · 2023-11-12T18:55:48Z

The already existing tests are successfully run, I've yet to write some tests which have the new angular control flow. I'll do it early next week.

…low syntax

ssougnez · 2023-11-17T15:43:34Z

Hello,

any updates on this pull request?

More and more developers are migrating to Angular 17 and the new syntax is really counterproductive without correct indentation.

I'm sure you guys have a lot of work, so take this as a gentle reminder 🥇

gergely-gyorgy-both · 2023-11-17T17:25:57Z

Hello,

any updates on this pull request?

More and more developers are migrating to Angular 17 and the new syntax is really counterproductive without correct indentation.

I'm sure you guys have a lot of work, so take this as a gentle reminder 🥇

It will sure take some time until this formatting improvement is usable in VSCode. Until then, personally, I decided not to use the new syntax. While I've upgraded to Angular 17, I have postponed the migration of the control flow syntax. Good thing is that until then, the old syntax can be used.

Also, if the review comes in, I'll fix the findings as soon as possible.

ssougnez · 2023-11-17T17:36:34Z

That's what I did as well but the new control flow seems very exciting :-)

I'll be patient, thanks

NachmanBerkowitz · 2023-11-30T19:50:30Z

Will this be merged anytime soon? If not I guess I can look at Prettier.

bitwiseman · 2023-11-30T21:35:04Z

js/src/html/tokenizer.js

+    return token;
+  }
+
+  if (c === '@' && /[a-zA-Z0-9]/.test(this._input.peek(1))) {


NOTE: This is on by default for HTML users. This has the possibility of causing significant breakage for some users.

We need to limit the chances of this triggering when not intentional.

This should look for a known list of control flow words.
https://angular.io/guide/control_flow seems to indicate the following list: @if, @else, @for, @empty, @switch, @case, @default.

You probably should us the pattern facilities already present in the library. This section shows who CDATA sections are checked for and read.

https://github.com/beautify-web/js-beautify/blob/1df08a0ca57b0c6a728223bbfd61e3ee81f447fe/js/src/html/tokenizer.js#L155-L165

The cdata pattern checks if the starting regex is present and then reads until it is finds the closing regex:

cdata: pattern_reader.starting_with(/<!\[CDATA\[/).until_after(/]]>/),

You don't need to read the whole control string using a Pattern but you can use to provide the opening guard. Something like:

angular_control_flow: pattern_reader.starting_with(/\@(if|else|for|empty|switch|case|default)[ ]*\(/).until(/\(/),

Keep the check for c === '@' to improve performance, but then call read() on that pattern. If it returns empty string, you have no match.

Note that there are other keywords: https://angular.io/guide/defer

@ssougnez
Okay, I guess it's pretty obvious I'm obviously not an Angular user.

A more flexible pattern might be:

pattern_reader.starting_with(/\@[a-zA-z]+[ ]*/).until(/[({]/),

Still I'd rather lean toward a specific list of @-strings if possible.

As an alternative, I'd also be okay with having this handler be off by default but that has it's own problems.

To answer this question, we should have a look to see if it's possible to create custom control flow directives. It is possible to create custom structural directives (which are the "old" way of doing control flow) so it's not impossible that it is (will be) possible for control flow as well.

Even though I understand that a white list is more secure and easy to test/maintain, this would break if (when) angular allow this.

To answer this question, we should have a look to see if it's possible to create custom control flow directives. It is possible to create custom structural directives (which are the "old" way of doing control flow) so it's not impossible that it is (will be) possible for control flow as well.

Even though I understand that a white list is more secure and easy to test/maintain, this would break if (when) angular allow this.

That was my exact thought when I've written the code.

NOTE: This is on by default for HTML users. This has the possibility of causing significant breakage for some users.

We need to limit the chances of this triggering when not intentional.

This should look for a known list of control flow words. https://angular.io/guide/control_flow seems to indicate the following list: @if, @else, @for, @empty, @switch, @case, @default.

You probably should us the pattern facilities already present in the library. This section shows who CDATA sections are checked for and read.

https://github.com/beautify-web/js-beautify/blob/1df08a0ca57b0c6a728223bbfd61e3ee81f447fe/js/src/html/tokenizer.js#L155-L165

The cdata pattern checks if the starting regex is present and then reads until it is finds the closing regex:

cdata: pattern_reader.starting_with(/<!\[CDATA\[/).until_after(/]]>/),

You don't need to read the whole control string using a Pattern but you can use to provide the opening guard. Something like:

angular_control_flow: pattern_reader.starting_with(/\@(if|else|for|empty|switch|case|default)[ ]*\(/).until(/\(/),

Keep the check for c === '@' to improve performance, but then call read() on that pattern. If it returns empty string, you have no match.

Your main point is that this beautifier is used in every HTML file, even if it's not an Angular file. That's why I added a new templating option. If angular is not set, then no control flow token will be set (so everything will remain as it was before this modification). If angular is set, then everything which is written after an @ character, is treated as a control flow phrase from Angular's side.

So it makes no sense to only match for the reserved keywords, as for Angular, everything is treated as control flow from Angular's side (even if it is a non-existing one).

But let me look into your other suggestions, I'll try to respond today.

pattern_reader.starting_with(/\@[a-zA-z]+[ ]*/).until(/[({]/),

This solution marks only the @ character and the control flow's name, but not the whole expression until it's opening curly brace.

Example:

@if(condition) {

The problem with this is that the
@if(condition) {
part would be treated as simple text.
If the opening curly brace would be written in a new line, this would mess up the indentation.

 @if() { <div></div> }  @if() { <div></div> }

Before commiting the solution I wrote, I tried to write a pattern, which selects the whole control flow opening part.

@if(condition) {

If we see just this simple example, we could say "read everything untilAfter an { character". That works, but not in all cases.
The problem with this is that, taking the @if() { for example, there can be an unknown amount of curly braces and parentheses inside the condition. So, if we had @if({value: true} !== undefined) {, then the pattern reader would only read until the first opening curly brace.

@if({value: true} !== undefined) {

This is, unfortunately, definitely not correct. And to select the whole expression via REGEX does not seem possible (at least not with the ES version this library uses). That's why I implemented the selection in pure JS, counting the pairs of opening and closing parentheses, until they're the same amount and we get to the opening curly brace character.

While selecting the whole expression as one token is not the best and most beautiful solution, breaking down this token to multiple ones (even, currently not existing tokens inside parentheses) would make this PR too big and would require a serious amount of time. By selecting the whole expression as one token, at least the indentation is solved which seems to be the biggest issue, and later we can work on some tweaks to have more formatting possibilities (e.g. opening curly brace in a new line, formatting the condition of the @if and so on)

Only a minor change request

The problem with this is that, taking the @if() { for example, there can be an unknown amount of curly braces and parentheses inside the condition. So, if we had @if({value: true} !== undefined) {, then the pattern reader would only read until the first opening curly brace.

I'm sorry I should have explained this better. I'm not asking you to read the whole token using one call to read(). I'm asking that you use the first call to read() to make your matching check stronger.

I think the guard pattern we want is pattern_reader.matching(/\@[a-zA-z]+[\n\t ]*[({]/). This will only read if it finds this pattern, otherwise it returns blank and does not advance the input scanner. If it returns a matching string, then you can continue with the rest of your code with only minor changes.

Suggested change

if (c === '@' && /[a-zA-Z0-9]/.test(this._input.peek(1))) {

// add to the __patterns initialization around line 72:

// this.__patterns.angular_at_string_start = pattern_reader.matching(/\@[a-zA-z]+[\n\t ]*[({]/),

if (c === '@') {

resulting_string = this.__patterns.angular_at_string_start.read();

if (resulting_string === '') {

return token; // null, not a control flow at string

}

var opening_parentheses_count = resulting_string.endsWith('(') ? 1 : 0;

var closing_parentheses_count = 0;

// [ ... the rest of your implementation ...]

On by default for HTML

Your main point is that this beautifier is used in every HTML file, even if it's not an Angular file. That's why I added a new templating option. If angular is not set, then no control flow token will be set (so everything will remain as it was before this modification).

It looks like this is what your code does, but it contradicts the behavior described for the option:

--templating [...] auto = none in JavaScript, all in html ^^^^^^^^^^^

What are our other options?

Add a new option called indent-templating. It would be a selection list like templating but would default to none and have only two valid values handlebars and angular. It would deprecate the current indent_handlebars setting - if that is set it adds handlebars to indent-templating.

Do a major version bump and make some long needed changes including changing templating to default to none. Then we wouldn't have to worry about this causing unexpected effects.

I'm sorry I should have explained this better. I'm not asking you to read the whole token using one call to read(). I'm asking that you use the first call to read() to make your matching check stronger.

I think the guard pattern we want is pattern_reader.matching(/\@[a-zA-z]+[\n\t ]*[({]/). This will only read if it finds this pattern, otherwise it returns blank and does not advance the input scanner. If it returns a matching string, then you can continue with the rest of your code with only minor changes.

I see, I didn't think of writing a pattern which doesn't match the whole token, but only a part of it. I'll do it asap. Thanks for the explanation!

It looks like this is what your code does, but it contradicts the behavior described for the option:

--templating [...] auto = none in JavaScript, all in html ^^^^^^^^^^^

Yes, I saw that this option will be on by default. I just haven't thought that this is the scope of this PR. But I understand that why it could cause conflicts and frustrate the users.

I can agree with both of your recommendations. The first sound like a quick fix, but it doesn't sound like a real solution, just a bandage to the current implementation. The latter is what I'd fancy, but it seems like that would require a bigger modification.

A follow-up task/issue should be to make those long needed changes to the templating option, then we could tokenize better. I'd be happy to do it in the future, but I cannot make any promises, that's why implementing the safer option now sounds a bit better.

What really bothers me in the current setup is that the @, {, } characters became reserved characters in angular (so you cannot use it in any other context, but only for control flow blocks), but they are normal characters in a non-angular scenario...

bitwiseman · 2023-11-30T22:26:28Z

test/data/html/tests.js

@@ -3839,6 +3839,304 @@ exports.test_data = {
        '</span>'
      ]
    }]
+  }, {


These are a solid set of tests. We need a couple more:

do not indent an unrecognized @-string - @foo (true) { should be treated as regular content.

do not indent invalid syntax - @if [] { is missing the required open parenthesis, treat as regular content.

do not match escaped @ symbol - I don't know how angular escapes, but ...

do not match if @-string is not preceded by } or whitespace (including newline)

do not match if @-string is not preceded by } or whitespace (including newline)

For this, could you please provide an example of what you mean?

I believe this is valid angular control flow and should match and be indented:

FYI: I want you to know that @if (a > b) { {{a}} is greater than {{b}}. }@else{ {{a}} is less than or equal to {{b}}. } Or to say it another way, {{b}} @if (a > b) {is less than}@else{is greater than or equal to} {{a}}.

There's a bunch of missing spaces, but the @-strings are always preceded by a space (tab or newline would also be okay), or a }.

The following might be valid control flow but are too close to other strings you might see in non-angular scenarios, so do not match or indent:

My email is [email protected] (only for work). bitwiseman@if (true) { }

My email is [email protected] (only for work). bitwiseman@if (true) { }

For this, the code is not selecting any control flow token, as the TK_TEXT token selector is not being cut when a @ character is found, only at spaces.

Or to say it another way, {{b}} @if (a > b) {is less than}@else{is greater than or equal to} {{a}}.

For this, I've found a bug in my code: the @if (a > b) { is registered as a control flow opening token, but the } character is not as a closing one (because it is a part of the than}@else{is TK_TEXT token.
When we implement the improvement of the templating option, we can improve the selectors and register the tokens in a better way, but for now I'm planning to just check for a } character : if currently a control flow is opened, it's a control flow closing character. Any other way: it's just text. I'm doing it today-tomorrow also.

@bitwiseman I tried a lot of different approaches, but because the special characters ({, }, @) can have a lot of meanings in different templating languages (@ may be just a character, while it's a reserved character in Angular; { and } may be block opening characters, handlebar characters or just simple characters), it's simply just not possible to make a 100% percent match for those characters, especially if they are not preceded by a whitespace character in the current implementation.
Example:
We'd like to match this as an angular control flow syntax: {{b}} @if (a > b) {is less than}@else{is greater than or equal to} {{a}}.

But we'd like to not match the following as an angular control flow syntax: bitwiseman@if (true) { }.

Unfortunately, by code I cannot differentiate the two cases if it is unknown what templating language we are using. A really good match could be achieved if we could aim only for the specific templating language, hope we can do it one day, as you suggested.

bitwiseman

This is a high quality PR! good tests, well implemented.

A few changes requested, mostly in the interest of avoiding breaking other users.

gergely-gyorgy-both · 2023-12-03T15:17:49Z

So, I've written a lot of stuff here, but to summarize:
I've changed the selection of the @-syntax, now it is done partly by a pattern. I've also added a test case not to select the @if() {, if it is not preceded by a whitespace character.
Unfortunately, I don't have now enough time to revise the code and change the templating to none by default. Please let me know what else you'd like to be done in this PR.
This modification should not cause conflicts to existing users (I doubt that a lot of users use @something { in the code in a non-angular context) and the indenting of angular files should be OK. A lot to be done here when the templating gets set to none by default.

…tespace

gergely-gyorgy-both · 2023-12-04T07:21:30Z

Or to say it another way, {{b}} @if (a > b) {is less than}@else{is greater than or equal to} {{a}}.
For this, I've found a bug in my code: the @if (a > b) { is registered as a control flow opening token, but the } character is not as a closing one (because it is a part of the than}@else{is TK_TEXT token. When we implement the improvement of the templating option, we can improve the selectors and register the tokens in a better way, but for now I'm planning to just check for a } character : if currently a control flow is opened, it's a control flow closing character. Any other way: it's just text. I'm doing it today-tomorrow also.

I've come up with a solution for this: if we have a control flow syntax open token, then we know that the } character, taking into consideration that the handlebars ({{ and }} characters) are already tokenized when we are parsing through words, cannot be anything else but the control flow's closing token. If we don't have an opening control flow token, I'm treating it as just a normal character in the text token.

Just one sidenote to your idea, saying that we should change the indent_handlebars settings:
In Angular, handlebars also function as special characters, so tokenizing them as TAG_OPEN and TAG_CLOSE is necessary for this control flow to work. The good news is that it seems that they don't conflict (the handlebars and the angular templating, I mean). As the indent_handlebars setting has to be set to ON for this control flow indentation to work, I suggest just renaming that to something like indent_handlebars_and_angular. This way, it will be more verbose for users to know that it has to be turned on for the indentation to work in Angular.

NachmanBerkowitz · 2023-12-04T16:10:41Z

Thanks for all the hard work.

js/src/html/tokenizer.js

bitwiseman · 2023-12-07T03:55:07Z

js/src/html/tokenizer.js

    single_quote: templatable_reader.until_after(/'/),
    double_quote: templatable_reader.until_after(/"/),
    attribute: templatable_reader.until(/[\n\r\t =>]|\/>/),
    element_name: templatable_reader.until(/[\n\r\t >\/]/),

+    angular_control_flow_start: pattern_reader.matching(/\@([a-zA-z]+[\n\t ]*)+[({]/),


@gergely-gyorgy-both
the problem looks like you wrote [a-zA-z] (lowercase second z) and what you wanted was [a-zA-Z] (uppercase second z). Otherwise, ths was fine.

Well, it was not selecting every case anyways :)

…en if indent_handlebars is true

bitwiseman · 2023-12-07T04:02:15Z

@gergely-gyorgy-both
Please don't burn yourself out on this. You've done great work. Let's me take a swing at this and see if I can come up with something.

I do have one request: if you can, try to avoid force pushing. I do not care about a clean commit history inside a PR, it is much more important to be able to backtrack to a previous commit and see the progression of changes. If I can see the whole commit history, I have a better chance of being able to help.

Thanks again for all your hard work on this.

gergely-gyorgy-both · 2023-12-07T05:40:14Z

@gergely-gyorgy-both Please don't burn yourself out on this. You've done great work. Let's me take a swing at this and see if I can come up with something.

I do have one request: if you can, try to avoid force pushing. I do not care about a clean commit history inside a PR, it is much more important to be able to backtrack to a previous commit and see the progression of changes. If I can see the whole commit history, I have a better chance of being able to help.

Thanks again for all your hard work on this.

Sorry for the force pushing, I'll avoid it in the future. I'm curious what you will come up with. Sorry for pushing commits which result in a failing pipeline.

Anyway, the code now works, doesn't affect any non-angular users. I don't feel burnt out, it just took me time until I could write a code that actually works. Interesting that even Angular's own VSCode plugin has problems with this syntax.

gergely-gyorgy-both · 2023-12-08T07:38:05Z

I couldn't stop thinking why the build fails, because I didn't modify anything except the html beautifier, the tokenizer and the html test file since the last successful build. It seems like that for some reason, when the make creates the js-beautify folder in build/node_modules, the src folder doesn't get created there, but the script looks for files inside it. When copying the src folder there manually, the build succeeds.

antur84 · 2023-12-18T17:37:27Z

I couldn't stop thinking why the build fails, because I didn't modify anything except the html beautifier, the tokenizer and the html test file since the last successful build. It seems like that for some reason, when the make creates the js-beautify folder in build/node_modules, the src folder doesn't get created there, but the script looks for files inside it. When copying the src folder there manually, the build succeeds.

It's the build action for the project itself that's broken down, unrelated to this PR. Same error in all branches if I look at the github "Actions" tab.

bitwiseman · 2024-01-03T01:08:04Z

I couldn't stop thinking why the build fails, because I didn't modify anything except the html beautifier, the tokenizer and the html test file since the last successful build. It seems like that for some reason, when the make creates the js-beautify folder in build/node_modules, the src folder doesn't get created there, but the script looks for files inside it. When copying the src folder there manually, the build succeeds.

It's the build action for the project itself that's broken down, unrelated to this PR. Same error in all branches if I look at the github "Actions" tab.

Doh. It looks like anew version of npm broke things. 😭 Fixed now in main.

Needs final review

thaoula · 2024-01-17T00:05:51Z

Hi team,

Many thanks for your hard work on this pull request. I think the entire Angular ecosystem is waiting for this awesome work to be merged.

Does anyone know what the process or timeline would be in getting this into vscode asap.

Regards,
Tarek

liesahead · 2024-01-23T08:18:38Z

@bitwiseman , could you provide some estimation for your review please?

NachmanBerkowitz · 2024-01-30T18:18:38Z

Is this happening?

js/src/html/tokenizer.js

Limiting this to a smaller set.

bitwiseman · 2024-01-31T19:38:56Z

@liesahead @NachmanBerkowitz @thaoula @antur84
I had concerns about this change (that I've already mentioned to @gergely-gyorgy-both) related to side effects on regular HTML files. But I see he's made this functionality on turn on if indent_handlebars is enabled. While a little it is a little odd, it is a reasonable way to ensure people only see this functionality if they opt-in to it.

@gergely-gyorgy-both
I've made one change to the detection regex to limit it to a smaller set of matches - @[a-zA-Z].

liesahead · 2024-01-31T20:07:33Z

@bitwiseman , thanks for finding the time to review this. I understand that it is a risky one (as it targets specific framework), so understandable it takes much time. And thanks @gergely-gyorgy-both for doing so much work on this.

gergely-gyorgy-both · 2024-01-31T20:34:46Z

@liesahead @NachmanBerkowitz @thaoula @antur84 I had concerns about this change (that I've already mentioned to @gergely-gyorgy-both) related to side effects on regular HTML files. But I see he's made this functionality on turn on if indent_handlebars is enabled. While a little it is a little odd, it is a reasonable way to ensure people only see this functionality if they opt-in to it.

@gergely-gyorgy-both I've made one change to the detection regex to limit it to a smaller set of matches - @[a-zA-Z].

First of all, thanks for re-reviewing this PR. While indent-handlebars was used before for something else, it gets the job done here, and maybe the naming can also stay as it is (as we are actually indenting content between handlebars).
Your regex change is reasonable, I couldn't find anything official about what characters can an opening token contain.
If I take a look at the VSCode hightlight (if the official Angular extension is installed), I can see that, for example, any number character can be in the token (the token is purple, so it's recognized as one).

There cannot be some characters (e.g. ^)

but there can be other, non-azAZ characters (e.g. japanese, chinese, arabic, etc.).

For now, it seems that no custom token can be made, so only those are in use what Angular predefined (and they only contain normal characters, so all's OK for now), but when(/if) they make it possible to create custom ones, we might have to change back the regex to the one which recognizes non-azAZ characters after the @.

Again, thank you for your efforts in reviewing this PR and helping me.

meowzhin · 2024-02-04T20:34:37Z

Whoa, this is finally happening? :D

liesahead · 2024-02-09T08:34:11Z

Was it forgotten again? 😿

gergely-gyorgy-both · 2024-02-09T08:45:34Z

Was it forgotten again? 😿

It's not up to me, and I'm pretty sure that @bitwiseman is very busy. If there's anything to do, I'll try to respond quickly, but we should be patient.

liesahead · 2024-02-09T08:47:37Z

Was it forgotten again? 😿

It's not up to me, and I'm pretty sure that @bitwiseman is very busy. If there's anything to do, I'll try to respond quickly, but we should be patient.

I understand both of these points, just keeping this thread alive so this (piece of cake) work wouldn't be abandoned.

meowzhin · 2024-02-15T11:18:19Z

Was it forgotten again? 😿

It's not up to me, and I'm pretty sure that @bitwiseman is very busy. If there's anything to do, I'll try to respond quickly, but we should be patient.

Yeah, I meant no pressure, sorry. This is really something that will ease my mind while "angularing" on vs code. 🕺🏼

bitwiseman · 2024-02-15T17:27:21Z

Sorry for the delay folks.

ssougnez · 2024-02-15T17:28:34Z

Hoorayyyyyyyyy

Thanks a lot :-)

NachmanBerkowitz · 2024-02-15T18:22:21Z

Thank you. Is there any config necessary to get it to work?

* Format index.html * Fix package.json files config * Ignore Pythong .eggs directory * update strings for move from beautify-web to beautifier org * Bump actions/setup-python from 4 to 5 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> * Bump github/codeql-action from 2 to 3 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2 to 3. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@v2...v3) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> * Update codemirror and other web page depenedencies (#2234) * Fixed #2219 - formatting of new Angular control flow syntax (#2221) * Fixed #2219 - formatting of new Angular control flow syntax * Add 'angular' templating option; use it for html beautifier control flow syntax * Add more precise selection for angular control flow close tag * Print angular control flow tokens with basic formatting * Add tests for fixing issue #2219 * Change angular control flow selection to do via pattern * Fix selecting control flow closing brace if it is not preceded by whitespace * Fix regex for control flow start pattern; only select control flow open if indent_handlebars is true * Changing angular at-string detection regex Limiting this to a smaller set. --------- Co-authored-by: Liam Newman <[email protected]> * Update Changelog * Bump version numbers for 1.15.0 * Release: 1.15.0 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Liam Newman <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: gergely-gyorgy-both <[email protected]> Co-authored-by: GitHub Action <[email protected]>

gergely-gyorgy-both force-pushed the fix-angular-control-flow-syntax-formatting branch 2 times, most recently from 90afe0a to 7c96d48 Compare November 12, 2023 18:48

gergely-gyorgy-both added 4 commits November 13, 2023 16:53

Fixed beautifier#2219 - formatting of new Angular control flow syntax

616304c

Add 'angular' templating option; use it for html beautifier control f…

cf6a64e

…low syntax

Add more precise selection for angular control flow close tag

1867e24

Print angular control flow tokens with basic formatting

08ecdd3

gergely-gyorgy-both force-pushed the fix-angular-control-flow-syntax-formatting branch from 1756232 to 08ecdd3 Compare November 13, 2023 16:09

Add tests for fixing issue beautifier#2219

1ffd900

gergely-gyorgy-both marked this pull request as ready for review November 15, 2023 14:24

bitwiseman reviewed Nov 30, 2023

View reviewed changes

bitwiseman previously requested changes Nov 30, 2023

View reviewed changes

Change angular control flow selection to do via pattern

3d53f6d

Fix selecting control flow closing brace if it is not preceded by whi…

cc5429f

…tespace

github-advanced-security bot found potential problems Dec 5, 2023

View reviewed changes

Fix regex for control flow start pattern; only select control flow op…

8900b86

…en if indent_handlebars is true

gergely-gyorgy-both force-pushed the fix-angular-control-flow-syntax-formatting branch from 4014fd6 to 8900b86 Compare December 6, 2023 12:20

Merge branch 'main' into fix-angular-control-flow-syntax-formatting

258617d

Merge branch 'main' into fix-angular-control-flow-syntax-formatting

8f7a35c

bitwiseman self-requested a review January 5, 2024 00:30

Merge branch 'main' into fix-angular-control-flow-syntax-formatting

731c977

bitwiseman reviewed Jan 31, 2024

View reviewed changes

js/src/html/tokenizer.js Outdated Show resolved Hide resolved

Changing angular at-string detection regex

0deb300

Limiting this to a smaller set.

bitwiseman approved these changes Feb 15, 2024

View reviewed changes

bitwiseman merged commit 5aff965 into beautifier:main Feb 15, 2024
12 checks passed

bitwiseman added this to the v1.15.0 (next release) milestone Feb 15, 2024

bitwiseman modified the milestones: v1.15.1 (next release), v1.15.0 Feb 15, 2024

thaoula mentioned this pull request Feb 20, 2024

Support for Angular control flow formatting microsoft/vscode#205651

Closed

-  if (c === '@' && /[a-zA-Z0-9]/.test(this._input.peek(1))) {
+  // add to the __patterns initialization around line 72:
+  // this.__patterns.angular_at_string_start = pattern_reader.matching(/\@[a-zA-z]+[\n\t ]*[({]/),
+  if (c === '@') {
+    resulting_string = this.__patterns.angular_at_string_start.read();
+    if (resulting_string === '') {
+      return token; // null, not a control flow at string
+    }
+    var opening_parentheses_count = resulting_string.endsWith('(') ? 1 : 0;
+    var closing_parentheses_count = 0;
+    // [ ... the rest of your implementation ...]

Fixed #2219 - formatting of new Angular control flow syntax #2221

Fixed #2219 - formatting of new Angular control flow syntax #2221

Conversation

gergely-gyorgy-both commented Nov 11, 2023 • edited Loading

Description

Before Merge Checklist

gergely-gyorgy-both commented Nov 11, 2023 • edited Loading

gergely-gyorgy-both commented Nov 12, 2023

gergely-gyorgy-both commented Nov 12, 2023

ssougnez commented Nov 17, 2023

gergely-gyorgy-both commented Nov 17, 2023 • edited Loading

ssougnez commented Nov 17, 2023

NachmanBerkowitz commented Nov 30, 2023

bitwiseman Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gergely-gyorgy-both Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

gergely-gyorgy-both Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

bitwiseman Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Only a minor change request

On by default for HTML

What are our other options?

gergely-gyorgy-both Dec 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

gergely-gyorgy-both commented Dec 3, 2023 • edited Loading

gergely-gyorgy-both commented Dec 4, 2023 • edited Loading

NachmanBerkowitz commented Dec 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitwiseman commented Dec 7, 2023

gergely-gyorgy-both commented Dec 7, 2023

gergely-gyorgy-both commented Dec 8, 2023

antur84 commented Dec 18, 2023

bitwiseman commented Jan 3, 2024

thaoula commented Jan 17, 2024

liesahead commented Jan 23, 2024

NachmanBerkowitz commented Jan 30, 2024

bitwiseman commented Jan 31, 2024

liesahead commented Jan 31, 2024

gergely-gyorgy-both commented Jan 31, 2024

meowzhin commented Feb 4, 2024

liesahead commented Feb 9, 2024

gergely-gyorgy-both commented Feb 9, 2024

liesahead commented Feb 9, 2024

meowzhin commented Feb 15, 2024

bitwiseman commented Feb 15, 2024

ssougnez commented Feb 15, 2024

NachmanBerkowitz commented Feb 15, 2024

gergely-gyorgy-both commented Nov 11, 2023 •

edited

Loading

gergely-gyorgy-both commented Nov 11, 2023 •

edited

Loading

gergely-gyorgy-both commented Nov 17, 2023 •

edited

Loading

bitwiseman Nov 30, 2023 •

edited

Loading

gergely-gyorgy-both Dec 1, 2023 •

edited

Loading

gergely-gyorgy-both Dec 1, 2023 •

edited

Loading

bitwiseman Dec 1, 2023 •

edited

Loading

gergely-gyorgy-both Dec 2, 2023 •

edited

Loading

gergely-gyorgy-both commented Dec 3, 2023 •

edited

Loading

gergely-gyorgy-both commented Dec 4, 2023 •

edited

Loading