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

Add separate CodeFormatter.FormatDocumentAsync overloads with cursor and config #2763

Merged
merged 4 commits into from
Feb 4, 2023

Conversation

DedSec256
Copy link
Contributor

/// <param name="cursor">The location of a cursor, zero-based.</param>
static member FormatDocumentAsync:
isSignature: bool * source: string * ?config: FormatConfig * ?cursor: pos -> Async<FormatResult>
isSignature: bool * source: string * cursor: pos -> Async<FormatResult>
Copy link
Contributor

Choose a reason for hiding this comment

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

This overload seems excessive and I don't see it being used in practice.
If you want to use this then you should use the next one and pass the default config.
Please remove it.

CHANGELOG.md Outdated
@@ -1,5 +1,10 @@
# Changelog

## [Unreleased]
Copy link
Contributor

Choose a reason for hiding this comment

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

There was no way you could have known this but for the non-main branch, I prefer that you don't add a changelog entry. This makes rebasing the v6 branch with the main branch easier. (fewer conflicts).

config,
?cursor = cursor
)
match cursor with
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you swap the order, please?
In this case, I prefer to have the shortest case first.

@DedSec256 DedSec256 force-pushed the dedsec256-overload-with-cursor branch from 6d8e95d to b990f9c Compare February 4, 2023 13:18
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Thanks!

@nojaf nojaf merged commit a5ea3b6 into fsprojects:v6.0 Feb 4, 2023
@nojaf
Copy link
Contributor

nojaf commented Feb 4, 2023

@DedSec256 available in https://www.nuget.org/packages/Fantomas.Core/6.0.0-alpha-003

@DedSec256 DedSec256 deleted the dedsec256-overload-with-cursor branch February 4, 2023 16:20
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.

2 participants