Skip to content

Regression in formatting for JS template strings #107571

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

Closed
kadircet opened this issue Sep 6, 2024 · 4 comments · Fixed by #119989
Closed

Regression in formatting for JS template strings #107571

kadircet opened this issue Sep 6, 2024 · 4 comments · Fixed by #119989

Comments

@kadircet
Copy link
Member

kadircet commented Sep 6, 2024

Actual formatting:

# clang-format a.ts

`${
    (
        FOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO -
            (barbarbarbar____bar_bar_bar_bar_bar_bar +
                bar_bar_bar_barbarbar___bar_bar_bar + 1),
        )}`;

Expected formatting:

`${
    (
        FOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO -
            (barbarbarbar____bar_bar_bar_bar_bar_bar +
             bar_bar_bar_barbarbar___bar_bar_bar + 1),
        )}`;

i.e. no extra indentation for the inner subexpression, as that's the formatting we got without template strings:

# clang-format a.ts

FOOOOOOOOFOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO -
    (barrrrbarbarbarbar____bar_bar_bar_bar_bar_bar +
     barrrrbar_bar_bar_barbarbar___bar_bar_bar + 1),
@kadircet
Copy link
Member Author

kadircet commented Sep 6, 2024

ccae7b4 seems to be the culprit

cc @gedare @owenca

@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/issue-subscribers-clang-format

Author: kadir çetinkaya (kadircet)

Actual formatting: ``` # clang-format a.ts

${ ( FOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO - (barbarbarbar____bar_bar_bar_bar_bar_bar + bar_bar_bar_barbarbar___bar_bar_bar + 1), )};


Expected formatting:

${ ( FOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO - (barbarbarbar____bar_bar_bar_bar_bar_bar + bar_bar_bar_barbarbar___bar_bar_bar + 1), )};


i.e. no extra indentation for the inner subexpression, as that's the formatting we got without template strings:

clang-format a.ts

FOOOOOOOOFOOFOOFOOFOO____FOO_FOO_FO_FOO_FOOO -
(barrrrbarbarbarbar____bar_bar_bar_bar_bar_bar +
barrrrbar_bar_bar_barbarbar___bar_bar_bar + 1),

</details>

@owenca
Copy link
Contributor

owenca commented Dec 14, 2024

@gedare can you look into this?

@gedare
Copy link
Contributor

gedare commented Dec 14, 2024

@gedare can you look into this?

Yes, I will.

gedare added a commit to gedare/llvm-project that referenced this issue Dec 14, 2024
The helper to check if a token is in a template string scans
too far backward. It should stop if a different scope is found.

Fixes llvm#107571
owenca pushed a commit that referenced this issue Dec 17, 2024
The helper to check if a token is in a template string scans too far
backward. It should stop if a different scope is found.

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

Successfully merging a pull request may close this issue.

5 participants