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
Maintain column indentation for list of functions #2446
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks, @cgravill for picking this up!
Would you mind adding in a Changelog entry?
https://github.com/fsprojects/fantomas/blob/master/CONTRIBUTING.md#changelog
You will probably need to add the Unreleased
header section.
And please include all duplicate issues in the changelog:
- Idempotency problem when Elmish Html #2195
- Splitting list results in code that doesn't compile #2201
- Idempotency problem when using a list with interpolated strings #2242
- Idempotency problem when using set and spread syntax #2392
There is no need to add tests for these duplicates as people should have added a space in the first place. But I'm considering all these closed and it should be recorded in the change log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, thanks for your patience.
CHANGELOG.md
Outdated
## [Unreleased] | ||
|
||
### Fixed | ||
* Maintain column indentation for list of functions. [#2158](https://github.com/fsprojects/fantomas/issues/2158) duplicates: [#2195](https://github.com/fsprojects/fantomas/issues/2195), [#2201](https://github.com/fsprojects/fantomas/issues/2201), [#2242](https://github.com/fsprojects/fantomas/issues/2242), [#2392](https://github.com/fsprojects/fantomas/issues/2392) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the title of the issue instead of the title of the PR.
Similar to all the examples below.
And put each issue on its own line, the duplicates:
is also somewhat strange compared to the rest.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done. I've not explicitly tested the others (as you requested above), but I've put them all on a separate line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I add them all to the fixes magic command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks, @cgravill!
Fixes #2158
I wasn't sure about the added duplication in
IndexWithoutDotExpr
but factoring the change seemed to add as much complexity. Elsewhere seems to favour mild duplication. Happy to adjust as preferred.Noticed in the ListTests.fs file the last function isn't marked as a test. I've checked and it runs fine. I have NOT added it to this PR but happy to or add a separate PR to get properly added and running:
fantomas/src/Fantomas.Core.Tests/ListTests.fs
Lines 2302 to 2321 in 2d5db3e