Support formatting of source selections

[pq]

Tool integrators will rejoice.

Minimally we'll want this for eclipse and IDEA/WS integration.

[munificent]

Can you give me some details on what input you want to pass to the formatter (is it just begin and end offsets for the selection range?) and what outputs (same)?

[pq]

Exactly right. That would be radical!

[pq]

Whoops. Wires crossed. This ask is about supporting the notion of formatting specific selected source in a file.

[stevemessick]

What I'd like to see is the ability to specify beginning and ending points of a region to be reformatted, the beginning and ending points of a region that is currently highlighted (in an editor), and the text. I'd like to get the same information back, adjust to be consistent with the changes to the text. I'm probably going to need this to close some bugs.

Allowing the regions to be lists of regions would be even better.

[alexander-doroshko]

Without this functionality WebStorm has to use own formatter (that is not that good) for cases like

• reformat code on paste
• reformat selection
• reformat only VCS changed code
• reformat generated code (getter/setter/constructor/etc)
• and many other cases

This feature looks very important for IDEs integration. Can it be scheduled for SDK 1.13?

[munificent]

Can it be scheduled for SDK 1.13?

I have a lot on my plate this month, so I can't make any promises, but I'll bump it up on my list of priorities.

[stevemessick]

When extending the API to cover this usage, consider allowing the indent level to be specified as an additional (optional) parameter.

[DanTup]

Is this still being considered? I've been working on the flutter repo lately and had to turn off format-on-save and format-on-type to avoid trashing their files (they have different guidelines - something I'm sure is a controversial topic!). This means formatting is being done somewhat manually (or automatically with then a bunch of manual editing).

It'd be really useful if I add a couple of new methods to a file to be able to run the formatter on just them (as a starting point) without affecting the rest of the file.

[munificent]

Yes, I would like to do this. I've just had zero time to hack on dartfmt lately. (Trying to get Dart 2 out the door and all...)

[DanTup]

I understand; wasn't a complaint it wasn't done, just a question. I was wondering whether to try and handle it client-side (eg. I thought I could use the selection start/end properties to be able to extract the formatted chunk from the full response - but I don't know how reliable it'd be?)

[srawlins]

Another use case for this: people who 99% like the formatter's output and wish they could use it 100% of the time, but have personal disagreements with it in those 1%. E.g. #731, #732, and half of the issues opened against the formatter.

The problem for people in this situation (those who have not "let go") is that dartfmt right now is a take-it-or-leave-it tool, You can't use it "most of the time," and clean up what you don't like about it (because dartfmt will wrench your hand-formatting back). If dartfmt could be invoked by a VCS-aware IDE, then it could be configured to only re-format new/changed lines. Then a user could "force-submit" their personal beef changes without dartfmt, then use dartfmt the rest of the time (99%).

I'm aware this all goes against design principles #1, #2, and #3. 😄 . Just a thought.

[matanlurey]

The biggest problem is Dartfmt needs to receive a completely valid program (it uses the full AST parser) so it's not really possible to select arbitrary lines to format. You might be able to, but you equally will not.

I am not sure we need to do anything here - folks that don't like the formatter will either not use it, or give up and use it. Potentially someone motivated could even write their own.

[srawlins]

I don't think it would be a huge loss if this issue was closed as not-gonna-happen, but other formatters have this capability. It should definitely be possible. For example:

void main() {              // line 1
  var line2;               // line 2
  var line3;               // line 3
  var line4 = something    // line 4
      .something           // line 5
      .something           // line 6
        .something(else)   // line 7
        .something(else);  // line 8
}                          // line 9

If the VCS reports that lines 7 and 8 are new/changed, and wants to reformat them, then dartfmt can do so easily, even though they are within a line-splitted statement. It can format the whole file in memory, then see that only lines 7 and 8 need to be changed anyway, so it can change them.

Here's a trickier example:

var a = b.method(() {  // line 1
  statement;
  statement;
  statement;
},);                   // line 5

Let's say that in this code, the only changed line is line 5: a comma was inserted! The code was previously formatted by dartfmt, and now the VCS-aware IDE says "line 5 was changed; gimme back the file with only line 5 formatted." This is impossible for dartfmt, since what it wants to do is this:

var a = b.method(
  () {
    statement;
    statement;
    statement;
  },
);  // Ahh, seven lines!

I think in this case, the formatter, when operating on source selections, would need to have a notion of statement boundaries or something, so that it says, "yeah, you told me to only format line 5, but line 5 is part of a statement that started on line 1, soooooo yeah I need to give you back new text for lines 1-5 (which is now lines 1-7). The same would have to be done for something like:

// BLANK LINE, USED TO BE `class A {`
  fn() { ... }

  // comment
  fn2() { ... }
// BLANK LINE, USED TO BE `}`.

We've deleted the class wrapping fn() and fn2(), so even though only lines 1 and 6 changed, dartfmt is going to say "Uh, I'm going to have to give you everything back from lines 1-6." Unfortunately this means that a change like:

class A implements B, C { // CHANGED LINE ADDING `, C`
  fn() { ... }

  fn2() { ... }
}

where only line 1 changed, adding a second implements class, will also force the formatter to say, "I know you only changed line 1 but, wow, I need to change lines 1-5 because I dunno what exactly happened; I just have to give you a whole new class text."

I just really really really really want this for when I contribute to Flutter. ☹️ . The flutter codebase is generally not dartfmt'd so my IDE goes bananas on every file I touch. Then I have the pleasure of turning off the formatter, undoing my change and redoing it ☹️ . But all this complexity is probably not worth it for that user story...

[matanlurey]

That problem really should be solved on their end - if they insist in bespoke formatting rules they should write a formatter. I was told that they have started accepting (?) dartfmt on new code, I could be wrong.

[DanTup]

The biggest problem is Dartfmt needs to receive a completely valid program (it uses the full AST parser) so it's not really possible to select arbitrary lines to format. You might be able to, but you equally will not.

I'd guess as long as the whole file is valid though, it could parse the AST but only emit changes within the selected range? Sure, it's weird to say you can't format line x because line y has a syntax error, but at least you can do it once you've fixed your code.

I've thought about trying to handle this directly in Dart Code by diffing the new contents against the current contents to break into smaller edits, then discard the ones outside the selected range. VS Code already added some code to break the diff up for us so that cursor locations are preserved, so maybe this wouldn't be too hard (another option may be for dart_style to return the smaller edits directly - assuming it can't already).

I haven't thought too much about it yet though because Dart-Code/Dart-Code#812 doesn't have many 👍's.

I just really really really really want this for when I contribute to Flutter.

FWIW, I contribute to Flutter too. I turn off auto-formatting for that workspace (set editor.formatOnSave, editor.formatOnPaste, editor.formatOnType to false in its Workspace Settings) and then tend to auto-format just before committing and then selectively select which lines to commit (in GitHub Desktop) and tweak them if required. It's not great, but the best I've come up with so far.

That problem really should be solved on their end - if they insist in bespoke formatting rules they should write a formatter.

There's also Dart-Code/Dart-Code#914 open in Dart Code about supporting (and I guess creating) a more flexible formatter. It's also currently not being looked at but it's there for people to put 👍's on to help gauge demand. If there's significant demand, I'll think a little more about what options there are.

The complication for a completely custom formatter though is that for performance reasons we want to use a formatter in the server (it already has ASTs for open files and doesn't have the overhead of spawning processes) but it's not replaceable. Maybe if there was clear demand (and a working alternative/prototype) it would be easier to have a discussion about that.

[munificent]

I think in this case, the formatter, when operating on source selections, would need to have a notion of statement boundaries or something, so that it says, "yeah, you told me to only format line 5, but line 5 is part of a statement that started on line 1, soooooo yeah I need to give you back new text for lines 1-5 (which is now lines 1-7).

It somewhat has that notion already because it handles preserving your selection when you format. It keeps track of where the new selection boundaries should be in the resulting code. That part isn't too hard (though I think there are some bugs in it).

The harder part is things like:

main() {
        // I like 8-space indents because why not.
        if (thing) {
                statement();
                another(); // line 5
        }
}

If the user tries to just reformat line 5, should it give them:

main() {
        // I like 8-space indents because why not.
        if (thing) {
                statement();
    another(); // line 5
        }
}

That's the correct level of indentation as far as dartfmt is concerned, but it doesn't exactly gel with the surrounding code.

At least in this example, the selection covered a single well-defined statement. Consider how weird it can get if their selection is in the middle of some complex expression with a lot of nesting and indentation.

In those cases, it's really hard to even define what the output should be.

That problem really should be solved on their end - if they insist in bespoke formatting rules they should write a formatter.

+1. If you want to push on someone, please consider pushing on the team that decided to come up with their own house style well after dartfmt had been shipped without offering any plan to migrate or deal with the millions of existing lines of code that had already been formatted with dartfmt.

[rakudrama]

Our presubmit checks demand that the file is formatted.
I'd like to run the formatter with the --fix-xxx flags but applied only to the edited elements.
If I edit a method, I want the tool to change the (existing) doc comment from /**...*/to ///, but leave the other comments alone.
I don't want the file edit history to blame the formatter fixes in unrelated code on my CL.

[natebosch]

I don't want the file edit history to blame the formatter fixes in unrelated code on my CL.

Opening a CL with only the formatting changes before my CL has worked well for me for years.

Alternatively --fix the entire codebase once and be done with it.