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

Reorder open statements #42

Closed
ovatsus opened this issue Jul 1, 2013 · 12 comments
Closed

Reorder open statements #42

ovatsus opened this issue Jul 1, 2013 · 12 comments

Comments

@ovatsus
Copy link

ovatsus commented Jul 1, 2013

It would be nice if fantomas reordered the open statements, similar to the reorder usings functionality in C#

@ghost
Copy link

ghost commented Jul 1, 2013

Presumably into alphabetical order.

@ovatsus
Copy link
Author

ovatsus commented Jul 1, 2013

Removing unused ones would be a nice extra, but I'm guessing it's much harder to implement, and alphabetical sort would be already very good

@dungpa
Copy link
Contributor

dungpa commented Jul 1, 2013

Sorting open statements is easy but removing unused ones is difficult.

The latter requires a type-checking phase which hasn't been implemented in Fantomas yet. I think the feature is in the scope of fsharp-refactor.

@ghost
Copy link

ghost commented Jul 1, 2013

Removing duplicates will be useful.

@ovatsus
Copy link
Author

ovatsus commented Jul 9, 2013

This was already done in #51

@ovatsus ovatsus closed this as completed Jul 9, 2013
@ovatsus
Copy link
Author

ovatsus commented Jul 12, 2013

Reopening, as I had to disable this because of #73
@dungpa any ideas how we can do this more safely

@ovatsus ovatsus reopened this Jul 12, 2013
@dungpa
Copy link
Contributor

dungpa commented Jul 12, 2013

We can add special treatments for open declarations in TokenMatcher module. The module should recognize the whole lines of open declarations and skip them although module names don't match.

@ovatsus
Copy link
Author

ovatsus commented Jul 12, 2013

What if there are comments between the open declarations? Before I disabled this feature, this:

//comment about System.IO
open System.IO
//comment about System
open System

Was being turned to this:

//comment about System.IO
open System
//comment about System
open System.IO

Is this acceptable or should we output this instead?

//comment about System
open System
//comment about System.IO
open System.IO

I'm gessing your proposed solution would output this:

//comment about System.IO
//comment about System
open System
open System.IO

Maybe this case is rare enough that even if we don't get it exactly right it's ok.

@dungpa
Copy link
Contributor

dungpa commented Jul 12, 2013

Yes, we are more likely to arrive at the last example. The second one is of course ideal.

I will try to improve TokenMatcher and see how far we can go.

@dungpa
Copy link
Contributor

dungpa commented Jul 12, 2013

@dsyme What do you suggest on this? It seems we can't reorder open statements without losing some comments around them.

@ovatsus
Copy link
Author

ovatsus commented Jul 15, 2013

@dungpa any progres on the TokenMatcher changes?

@dungpa
Copy link
Contributor

dungpa commented Jul 15, 2013

Not yet. I will work on that tomorrow.

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

No branches or pull requests

2 participants