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

Config: Empty formatter, no formatter #180

Merged

Conversation

andys8
Copy link
Contributor

@andys8 andys8 commented Jul 11, 2022

Hey,

Before looking it up, my first try to disable an existing formatter was to set the field to "". Later I found out there is "none".

I would also interpret "" as NoFormatter, but it's up to you if you think that's a good idea or not :)

@wclr
Copy link
Contributor

wclr commented Jul 12, 2022

The default formatter should be tidy nowdays. I think, any illegal value should be interpreted as no-formatter.

@andys8
Copy link
Contributor Author

andys8 commented Jul 12, 2022

Sounds good to me. @nwolverson What do you think?

@nwolverson
Copy link
Owner

Yep I agree, interpret anything other than the specific formatter strings as NoFormatter, something like vscode which has a default can set it to tidy

Every other value is interpreted as `NoFormatter`
@@ -198,12 +198,10 @@ formatter :: ConfigFn Formatter
formatter =
getString "formatter" ""
>>> case _ of
"none" -> NoFormatter
"" -> NoFormatter
"purty" -> Purty
"purs-tidy" -> PursTidy

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You tell me. Is tidy the more common name?

The npm package, the cli command and the announcement all use purs-tidy.

My preferred way would be to allow both tidy and purs-tidy since it's easy to mixup.

Copy link
Owner

Choose a reason for hiding this comment

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

As far as I'm aware that's its name.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright then

Copy link
Contributor

Choose a reason for hiding this comment

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

(interestingly both wclr and nwolverson you referred to it as tidy in your above comments :D)

@andys8
Copy link
Contributor Author

andys8 commented Jul 20, 2022

Is the PR content in it's recent state okay?

@@ -200,6 +200,7 @@ formatter =
>>> case _ of
"purty" -> Purty
"purs-tidy" -> PursTidy
"tidy" -> PursTidy
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@andys8 andys8 closed this Aug 17, 2022
@andys8 andys8 reopened this Aug 17, 2022
@andys8
Copy link
Contributor Author

andys8 commented Aug 17, 2022

Oops, wrong button 😅

@nwolverson nwolverson merged commit 8a70079 into nwolverson:master Sep 1, 2022
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.

4 participants