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

Different formatting around type annotations with JS #107401

Closed
kadircet opened this issue Sep 5, 2024 · 3 comments · Fixed by #107506
Closed

Different formatting around type annotations with JS #107401

kadircet opened this issue Sep 5, 2024 · 3 comments · Fixed by #107506
Assignees

Comments

@kadircet
Copy link
Member

kadircet commented Sep 5, 2024

a.js:

ctrl.onCopy(/** @type {!WizEvent}*/ (
    {event, targetElement: {el: () => selectedElement}}));

formatting after this change with clang-format -style='{AlignAfterOpenBracket: AlwaysBreak}' a.js

ctrl.onCopy(
    /** @type {!WizEvent}*/ (
        {event, targetElement : {el : () => selectedElement}}));

I can't really say this change looks like intended from the bug fix and code changes. Moreover, these kind of type annotations seems to be pretty common so unless the change deliberately changing behavior here can we restore the old behavior ?

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 5, 2024

@llvm/issue-subscribers-clang-format

Author: kadir çetinkaya (kadircet)

a.js: ``` ctrl.onCopy(/** @type {!WizEvent}*/ ( {event, targetElement: {el: () => selectedElement}})); ```

formatting after this change with clang-format -style='{AlignAfterOpenBracket: AlwaysBreak}' a.js

ctrl.onCopy(
    /** @<!-- -->type {!WizEvent}*/ (
        {event, targetElement : {el : () =&gt; selectedElement}}));

I can't really say this change looks like intended from the bug fix and code changes. Moreover, these kind of type annotations seems to be pretty common so unless the change deliberately changing behavior here can we restore the old behavior ?

@kadircet
Copy link
Member Author

kadircet commented Sep 5, 2024

cc @owenca @gedare @HazardyKnusperkeks

#93140 seems to be the change introducing this behavior

@owenca
Copy link
Contributor

owenca commented Sep 6, 2024

I can't really say this change looks like intended from the bug fix and code changes. Moreover, these kind of type annotations seems to be pretty common so unless the change deliberately changing behavior here can we restore the old behavior ?

It looks like unintended, and I have no object to restoring the old behavior.

owenca added a commit to owenca/llvm-project that referenced this issue Sep 6, 2024
@owenca owenca self-assigned this Sep 7, 2024
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this issue Sep 12, 2024
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.

4 participants