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

Sunset setting ReorderOpenDeclaration #645

Closed
nojaf opened this issue Jan 21, 2020 · 2 comments · Fixed by #707
Closed

Sunset setting ReorderOpenDeclaration #645

nojaf opened this issue Jan 21, 2020 · 2 comments · Fixed by #707
Labels

Comments

@nojaf
Copy link
Contributor

nojaf commented Jan 21, 2020

Description

The feature ReorderOpenDeclaration introduces a problem when two namespaces expose the same function.

For example

module TriviaTool.Client.App

open Elmish
open Elmish.React
open TriviaTool.Client
open Elmish.Debug
open Elmish.HMR

Program.mkProgram State.init State.update View.view
|> Program.withReactBatched "elmish-app"
|> Program.run

After formatting it becomes

module TriviaTool.Client.App

open Elmish
open Elmish.Debug
open Elmish.HMR
open Elmish.React
open TriviaTool.Client

Program.mkProgram State.init State.update View.view
|> Program.withReactBatched "elmish-app"
|> Program.run

The problem here is that Program.withReactBatched was used from the Elmish.HMR namespace initially but after formatting, it uses Elmish.React.

Another problem with reordering is trivia, consider the following example:

module TriviaTool.Client.App

// Super nice comment
open B
open A

after formatting this becomes:

module TriviaTool.Client.App

// Super nice comment
open A
open B

The comment is still linked to the first open statement after sorting, however, if the comment says something meaningful about the other statement that is now shuffled.

On top of my head, I don't see any way to resolve the first problem.
It will most likely be possible to change to solve the trivia case.

I'm in favour of dropping the functionality all together as it can lead to code not compiling anymore and might not fully be the responsibility of a code formatter.

Thoughts and opinions @jindraivanek?

Repro code

Fantomas online and second case

@jindraivanek
Copy link
Contributor

  • its true reorder open declarations can lead to compiler errors, or even to subtle bugs.
  • I agree with removal of ReorderOpenDeclaration, because it can break code (and I think Fantomas shoudln't break code in any setting).

@nojaf
Copy link
Contributor Author

nojaf commented Jan 22, 2020

Ok, I would approach this somewhat gradual:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants