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

Shift multiline paren contents less aggressively #16789

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Feb 29, 2024

Another followup to #16079.

Description

The logic in #16666 addressed some multiline parentheses removal issues, but it was slightly too aggressive and sometimes shifted lines farther than was necessary or safe.

For example, before #16666, the following code —

let longVarName1 = 1
let longVarName2 = 2
(
    longFunctionName
        longVarName1
        longVarName2
)

— when the unnecessary parentheses code fix was applied, would become —

let longVarName1 = 1
let longVarName2 = 2

    longFunctionName
        longVarName1
        longVarName2

— which was incorrect.

After #16666, it would become —

let longVarName1 = 1
let longVarName2 = 2
longFunctionName
longVarName1
longVarName2

— which was more correct in one way (longFunctionName needed to be shifted left), but less correct in another (the arguments to longFunctionName were shifted too far).

With this PR, it now correctly becomes:

let longVarName1 = 1
let longVarName2 = 2
longFunctionName
    longVarName1
    longVarName2

Checklist

  • Test cases added.
  • Release notes entry updated.

* The logic in dotnet#16666 addressed some multiline parentheses removal
  issues, but it was slightly too aggressive and sometimes shifted
  lines farther than was necessary or safe.
@brianrourkeboll brianrourkeboll marked this pull request as ready for review February 29, 2024 16:58
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner February 29, 2024 16:58
Copy link
Contributor

github-actions bot commented Feb 29, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
vsintegration/src docs/release-notes/.VisualStudio/17.10.md

Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Thanks!

@psfinaki
Copy link
Member

psfinaki commented Mar 4, 2024

/azp run

@psfinaki psfinaki enabled auto-merge (squash) March 4, 2024 12:57
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

Thanks!

@psfinaki psfinaki merged commit b33ad19 into dotnet:main Mar 4, 2024
31 checks passed
@brianrourkeboll brianrourkeboll deleted the parens-another-one branch March 4, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants