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

Format selected text only #2

Open
vasily-kirichenko opened this issue Apr 25, 2013 · 17 comments
Open

Format selected text only #2

vasily-kirichenko opened this issue Apr 25, 2013 · 17 comments

Comments

@vasily-kirichenko
Copy link

It'd be great to have an option to format the currently selected text only, not the entire file. Sometimes the code is formatted in a special way (test data formed as tree, for example) and it'd be good to keep that formatting.

@itowlson
Copy link
Owner

Hi Vasily, The Fantomas formatter doesn't currently support formatting a subset of the text (I believe because of whitespace issues) but I think it's on @dungpa's radar. I might take a shot at supporting Format Selection for 'simple' selections though - thanks for the feedback!

@dungpa
Copy link

dungpa commented Apr 25, 2013

Hi Vasily and Ivan,

I could expose a function for formatting selection. However, it needs careful UI handling so that the feature is used correctly. It's great if we have a thorough discussion before implementing this feature.

First, the selection has to be parsable by F# compiler. We can't select arbitrary text and hope F# compiler is able to parse it. Second, a selection has to form an expression, a type, a let binding, and the like. Many elements are identified based on their contexts; we have to be careful otherwise the loss of context will lead to wrong results. Third, F# constructs are whitespace sensitive so the selection hasn't to break indentation of the whole file. So I think the tool is able to support these constructs:

  • A single-line selection which forms an expression, a let binding, a module, a type declaration, etc.
  • A multi-line selection where the first line has the lowest indentation. It has to be an expression, a let binding, etc as single-line selection above.

What do you thinks about these? Do I miss any use case?

@vasily-kirichenko
Copy link
Author

It sounds good. And I'd add the third option:

  • Current nearest top-level expression (without any selection)

However, I suggest a different algorithm:

  • arbitrary selection (or the expression the caret is in at the moment) is "expanded" to the smallest parseable expression
  • the expression is formatted
  • the original expression is replaced in-place with the new one, respecting original indentation (I mean the orig file is not replaced entirely, but the formatted expr is)

It looks not a very easy thing to implement though.

@dungpa
Copy link

dungpa commented Apr 28, 2013

Indeed it is difficult to implement.

But your last point is a must. We only reformat the selection and have to plug the formatted code back into the original content. There are restrictions on the input to ensure that when we reformat selection, the output is correct wrt indentation.

Let's see what @itowlson comes up with :). He may have better ideas to make the implementation functional.

@dungpa
Copy link

dungpa commented May 9, 2013

@itowlson I've just published Fantomas v0.9.3 with supports for formatting selections.

You can see formatSelectionFromString at https://github.com/dungpa/fantomas/blob/master/src/Fantomas/CodeFormatter.fs#L94.

The function accepts a string and a range describing a selection. A selection could be a part of a line, a whole line or consisting of several lines of parsable F# code.

You can understand how to use the function through the testing module https://github.com/dungpa/fantomas/blob/master/src/Fantomas.Tests/FormattingSelectionTests.fs. It should be fairly easy to implement formatting selections now.

By the way, what is the status of implementing build scripts and bug fixing :-)?

@itowlson
Copy link
Owner

I've added a first cut of Format Selection support. Known issue: like Format Document, it currently resets the selection point to the end of the document. Fixing this is next on the agenda!

I do seem to be seeing an issue where I can't format top-level let forms e.g. in "let y = 1+2" it formats the "1+2" or "y = 1+2" substrings correctly, but gives a "indent level cannot be negative" error when I try to format the whole line. This appears to be a Fantomas limitation - I'll submit a repro over there and you can tell me if I'm missing something obvious!

@vasily-kirichenko
Copy link
Author

Good job, guys!

The same error - "indent level cannot be negative" - appears every time I try to format any top-level form (type, let, module).

And yes, the resetting of the selection point is really annoying now.

@itowlson
Copy link
Owner

Hi Vasily, I've committed a fix that should preserve the selection point across a Format Selection. Please let me know if you're still losing the selection in this case. Format Document is a bit trickier but I haven't forgotten about it!

@dungpa
Copy link

dungpa commented May 19, 2013

Hi Ivan and Vasily, I published Fantomas v0.9.4. It should fix the error with negative indent level. I try to make the fix as soon as possible so that you can focus on other issues in the Add-In.

Let me know if you find any problem. I will test fsharp-vs-commands next week to be sure I don't miss any obvious use case.

@vasily-kirichenko
Copy link
Author

Thanks, the selection is preserved now. However, a new 'wish' is suddenly appeared :) The document is scrolled to the top as the command is executed and I lose sight of the selection.

Another problem: if we select a form including heading white spaces, then the form is shifted to the left like this:

  1. Selection:
    image
  2. Result:
    image
    I think we should trim the selection before execute the command.

And it'd be great if the plugin can format the most inner expression/form in which the caret is in. For example, if you'd like to format a type, you just place the caret on the name of it or inside its default constructor signature and execute the command. If you'd like to format a simple expression like let x = 1, you place the caret at any place inside it, and so on.

@vasily-kirichenko
Copy link
Author

Just updated to 0.9.4. The bug with negative indent level has disappeared :)
However, there's a new bug. Having this module (from Fantomas itself :)

module ConsoleApplication2.Example

open System
open Fantomas.FormatConfig
open Fantomas.CodeFormatter

let config = { FormatConfig.Default with 
                IndentSpaceNum = 2 }

let source = "
    let Multiple9x9 () = 
      for i in 1 .. 9 do
        printf \"\\n\";
        for j in 1 .. 9 do
          let k = i * j in
          printf \"%d x %d = %2d \" i j k;
          done;
      done;;
    Multiple9x9 ();;"

Console.WriteLine("Input:\n{0}\n", source)
Console.WriteLine("Result:\n{0}\n", formatSourceString false source config)

Console.WriteLine("Press any key to finish...")
Console.ReadKey() |> ignore

Selection:
image
Error:
image

@itowlson
Copy link
Owner

Folded in 0.9.4

@dungpa
Copy link

dungpa commented May 20, 2013

@vasily-kirichenko I logged these two bugs and will try to fix them before weekend. Thanks for reporting. Next time, remember to attach the full code instead of a picture (just like you did in 2nd example). It would save me from typing the whole test case :-).

@dungpa
Copy link

dungpa commented May 20, 2013

@itowlson I forked the project and tested it on my machine (my first try with an C# project in a few years).

Great job so far. After bug fixing and adding build script, I would happily merge it to the Fantomas code base. Thanks for the awesome contributions.

@vasily-kirichenko
Copy link
Author

#2 is fixed, thanks!

@dungpa
Copy link

dungpa commented May 31, 2013

Hi Ivan,

Can you set a date for transferring the formatting command to Fantomas codebase? After testing the extension, I see that most of the functionalities are in. The remaining things are minor.

If there are some minor issues after merging, I will try to handle them before asking for your help :-). I will soon be occupied by new projects, therefore, I would like to deploy first version of the VS extension as soon as possible. It would help gather feedbacks, improvements and bug reports faster.

What do you think?

@dungpa
Copy link

dungpa commented Jul 1, 2013

@itowlson I publish the first release of the extension at http://visualstudiogallery.msdn.microsoft.com/24ef5c87-b4e3-4c3b-b126-1064cc66e148

Thank you again for your help.

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

3 participants