Skip to content

[SourceKit/CodeFormat] Column-align multiple patterns in catch clauses #30870

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

Merged

Conversation

nathawes
Copy link
Contributor

@nathawes nathawes commented Apr 8, 2020

catch clauses now support multiple patterns thanks to #27776. Like case patterns, these should be column-aligned if split across multiple lines like below:

do {
  ...
} catch MyErr.a(let x),
        MyErr.b(let x) {
  print("hello")
}

Resolves rdar://problem/61614223

@nathawes nathawes requested a review from benlangmuir April 8, 2020 04:35
@nathawes
Copy link
Contributor Author

nathawes commented Apr 8, 2020

@swift-ci please test

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

Thanks for updating this, the approach LGTM!

{
print("hello")
}
catch MyErr.a(let code. let message),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catch MyErr.a(let code. let message),
catch MyErr.a(let code, let message),

Comment on lines 2096 to 2098
if (!Elem.getPattern() || Elem.getPattern()->isImplicit())
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this indent something like the following correctly?

do {
  throw MyErr.a
} catch where foo == 0,
        where bar == 1 {
}

I don't think anyone will be writing code like this in practice, but it is allowed and may be worth testing

Copy link
Contributor Author

@nathawes nathawes Apr 8, 2020

Choose a reason for hiding this comment

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

Good catch! I'll fix it and add it to the test cases.

Catch clauses now support mutliple patterns. Like 'case' patterns, these
should be column-aligned if split across multiple lines.

do {
  ...
} catch MyErr.a(let x),
        MyErr.b(let x) {
  print("hello")
}
@nathawes nathawes force-pushed the mutiple-catch-pattern-indentation branch from 592d629 to 19d6eff Compare April 8, 2020 16:46
@nathawes
Copy link
Contributor Author

nathawes commented Apr 8, 2020

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Apr 8, 2020

Build failed
Swift Test Linux Platform
Git Sha - 592d629a1849f837f3a2600a1e3d02123e79d971

@swift-ci
Copy link
Contributor

swift-ci commented Apr 8, 2020

Build failed
Swift Test OS X Platform
Git Sha - 592d629a1849f837f3a2600a1e3d02123e79d971

@nathawes nathawes merged commit 13d5a8a into swiftlang:master Apr 11, 2020
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.

3 participants