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

New linter is going crazy on perfectly fine elmish code #296

Open
Tracked by #434
forki opened this issue Jan 28, 2019 · 12 comments
Open
Tracked by #434

New linter is going crazy on perfectly fine elmish code #296

forki opened this issue Jan 28, 2019 · 12 comments

Comments

@forki
Copy link
Member

forki commented Jan 28, 2019

Originally reported to ionide. Please see ionide/ionide-vscode-fsharp#997

@jgardella
Copy link
Contributor

Hi @forki.

This rule is also based on the F# style guide, which suggests always using parentheses around tuple instantiations. As @MangelMaxime suggested in the linked issue, it can be disabled through the config.

However if I'm understanding your comments on the linked issue correctly, it sounds like this is a common format in elmish/SAFE stack code? I was not aware of this. Given that this is in the official style guide, I think it makes sense to have it as a default. If elmish/SAFE code differs from the style guide in this instance or others, maybe we could create a config file specifically for elmish/SAFE projects?

@forki
Copy link
Member Author

forki commented Jan 28, 2019

Actually this is so established right now, that I think the "official style guide" needs to be changed. Adding parens here breaks basically all the historical code in light syntax and adds a lot of clutter. /cc @cartermp

@forki
Copy link
Member Author

forki commented Jan 28, 2019

I opened up a pr on the styleguide. dotnet/docs#10186

I don't want to be a dick here and will happily accept community decision.ut this one feels so completely anti-F# for someone like me who uses it like first inofficial code drops.

@jgardella
Copy link
Contributor

I think it's a great discussion to have, thanks for starting it. Personally, I like parentheses around tuple instantiation (and even around tuple pattern matching) as I think it makes the code easier to read. But again I have not used elmish/SAFE so I may not be seeing the full picture. Let's continue the discussion over on your style guide PR.

@MangelMaxime
Copy link

Considering the changes requested by @forki on the guidelines. If this is accepted, we could add a mode "strict" to FSharpLint for people who want to always put parens around their tuples.

The idea being because it's mentioned as optional people could want to force them self to always use parens even for function return.

@forki
Copy link
Member Author

forki commented Jan 28, 2019 via email

@MangelMaxime
Copy link

No idea, but just because we don't know doesn't me but can't provide an option to support it if some want to and because the code is already written for it. ^^

I believe in configuration against strict walls. Even, we good defaults a tool doesn't all suits the need for everyone.

@forki
Copy link
Member Author

forki commented Jan 28, 2019 via email

@cartermp
Copy link

I think having something like a requirement for all tuples to be surrounded by parentheses could make a good setting, but the default should be to allow them at the return site like the docs suggest in light of elmish styles.

@forki
Copy link
Member Author

forki commented Jan 29, 2019

Cool, so the suggestion to open up the restriction is merged to the styleguide.

@jgardella
Copy link
Contributor

Great, happy we were able to reach a decision and update the guide. I'm thinking for FSharpLint we can make this setting configurable with the following options:

  • Instantiation - require parentheses for all instantiations
  • InstantiationExceptReturn - require parentheses for all instantiations, except when they are return value (elmish style)

@MangelMaxime
Copy link

For me having both of these options make sense indeed.

@jgardella jgardella added this to the Version 1.0.0 milestone May 31, 2020
@jgardella jgardella mentioned this issue May 31, 2020
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants