[property editor] apply edits in a way that does not trigger a lint

[kenzieschmoll]

For example, there is a lint that the child or children parameter of a Widget should be listed last. When I added a value for a parameter that previously did not exist, the parameter was added to the end of the parameter list. I would have preferred that the parameter was added before child or children so that a lint warning was not triggered by the edit.

[elliette]

@DanTup has already made a change to add properties before child / children (but it didn't make the release cut-off): https://dart-review.googlesource.com/c/sdk/+/403584

However, there is still the issue of formatting any changes. Currently any added changes are unformatted, and there is no way to auto-format them with VS Code's settings.

To get changes made by the Property Editor to be saved, we currently need to enable VS Code's auto-save setting:

However, with auto-format is ignored for auto-save requests:

@DanTup @bwilkerson What are your thoughts on having the Analysis Server/IDE trigger an explicit format and save when applying a property edit? I think ideally we would want the user to be able to configure the behavior from the Property Editor itself (e.g. have a couple settings: "Automatically save edits" and "Automatically format edits")

[bwilkerson]

What are your thoughts on having the Analysis Server/IDE trigger an explicit format and save when applying a property edit?

If we have a setting that specifies that the user wants edits to be formatted, then I don't have a problem with doing so. We've talked before about having such a setting for the other edits the server makes (such as quick fixes or refactorings), but that would probably need to be in the analysis options file, and I'm not sure how the two would interact.

I don't think we should change the auto save setting. I don't know of any way for the server to request that a file be saved. Is there a problem if the changes aren't saved? Does it cause problems, for example, with hot reload?

[DanTup]

@DanTup @bwilkerson What are your thoughts on having the Analysis Server/IDE trigger an explicit format and save when applying a property edit?

I think I would be surprised if that happened by default (silently) if I hadn't enabled auto-format/save myself. However I can see the value in being able to have it hot-reload (which implies saving) as you make changes, so I think if there's some obvious toggle for the user to control this (and it's clear that it will happen), it's reasonable.

As for formatting - I think the first thing we should do is improve the generated code to be formatted better if it's particularly bad (I think right now maybe we don't ever put newlines in, but probably we should?). If we do need to run the real formatter, we do have some possible options we've discussed in the past but not implemented, for example by formatting the doc on the server and then computing the minimal edits for that change (we already have this) and then discarding things outside of a certain range (we already support format range). We would have to figure out what range to include in the formatting though - we don't want to format a users entire document when they change a single argument (especially if we can't be certain that they are using the formatter).

[bwilkerson]

However I can see the value in being able to have it hot-reload (which implies saving) as you make changes ...

Not being a Flutter developer I might be off base, but I would think that I'd prefer to be in control of when the changes were to be saved and reloaded. For example, if I want to reduce some padding in one location and increase it in another (that is, the goal is to shift the padding slightly), then I don't think it would be useful to see the intermediate step. I certainly wouldn't save half way through if I were editing the code directly.

We would have to figure out what range to include in the formatting ...

As a first approximation I'd suggest the constructor invocation whose argument(s) are being replaced.

That won't be sufficient in all cases, but it would be sufficient in most cases. If we want to be complete, then we could format the outermost constructor invocation.

But again, I don't think we should format the user's code without an explicit signal that that's what's desired, and if we want to go that route I'd like to consider the other cases for formatting before we decide what signal(s) to use.

[elliette]

Thanks for the input! Maybe the best solution would be to do the smallest necessary work for now, and then seeing what the user feedback is to decide whether to make this more configurable from the Property Editor.

For now, that would mean:

A) Informing the user how to update their settings in settings.json (or the equivalent in intelliJ):

• enable auto-saving
• enable hot-reloads on save
  This could be in a "Tips & tricks" section of the Property Editor, or in documentation on the Flutter site.

B) Per @DanTup's suggestion: "improve the generated code to be formatted better if it's particularly bad (I think right now maybe we don't ever put newlines in, but probably we should?)."

I do wonder if adding a newline break after the comma would resolve the majority of the formatting issues.
e.g. instead of mainAxisSize: MainAxisSize.min, we would insert mainAxisSize: MainAxisSize.min,\n

[kenzieschmoll]

I do wonder if adding a newline break after the comma would resolve the majority of the formatting issues.

We would also want to add the edit to a new line instead of appending to the end of an existing line. I noticed that we do not always do this.

[DanTup]

I've a change open that should improve the formatting when inserting arguments:

https://dart-review.googlesource.com/c/sdk/+/405122

If I understand correctly, that's the only outstanding thing here for now (the auto-save stuff sounds like it will initially just be documentation/recommendations and the child/children change was already made). Once that lands, I'd be interested in code samples if anyone still sees badly-formatted insertions (or other things triggering lints).