Open orestesgaolin opened 1 year ago
About specifying how much to extract. For Extract Local
refactoring we return to the client source ranges of containing expressions, so that IDE can ask the user. I think it works quite conveniently in IntelliJ.
It even highlights visually which portion it will extract.
We could do something like this here as well.
That's an interesting idea. I'm guessing that in the initial example that might look something like the following list:
SizedBox
SizedBox, ColoredBox
SizedBox, ColoredBox, Text
I'm also guessing that we'd stop before any widget with multiple widgets, because the number of combinations could be large and the options could be confusing.
What combinations? My understanding of the request is that we start at a child widget, and while keeping it where it is, extract a few layers of widgets around it. And if these layers reference other widgets, so let is be, these widgets will go into the extracted widget. And whatever variables these layers reference, will become formal constructor parameters of the new widget.
I was assuming that the user would select the top-most widget to be extracted, and that we needed to identify the lowest child being included in the extraction.
Starting at the bottom simplifies our task, but I don't know if that's how users would expect to use the feature.
Personally I don't have any preference, so if starting at the bottom makes things easier, then I don't see any reason to oppose it.
Maybe instead of supporting N layers of refactoring we suggest a few, like:
@johnpryan I don't understand what semantics you imply by saying "Extract 2 widgets and wrap with new widget". My understanding is that it would say "Extract 2 widgets as a new widget wrapping of the selected widget".
Anyway, I think visually expanding selection is better, rather than to force users to count.
I think visually expanding selection is better ...
It definitely would be (visual is almost always better than textual), but I don't know how we would highlight multiple widgets with a hole in the middle for the widget that we wouldn't be extracting.
@DanTup Is that something we can do using LSP (both highlighting two disjoint regions and providing multiple ranges that users can move between)?
If we can't show this visually then we'll need text that defines the range of widgets being extracted in a clear and concise way. That includes indicating whether the widgets are from the selection up the tree or from the selection down the tree.
I think visually expanding selection is better ...
It definitely would be (visual is almost always better than textual), but I don't know how we would highlight multiple widgets with a hole in the middle for the widget that we wouldn't be extracting.
It seems to be possible in IntelliJ.
I hacked IntroduceTargetChooser to extract -2
and +2
range from the middle of the expression range. Of course this cannot be used as is, but at least the platform allows this on lower levels.
If we can't show this visually then we'll need text that defines the range of widgets being extracted in a clear and concise way. That includes indicating whether the widgets are from the selection up the tree or from the selection down the tree.
We need these popup items anyway, highlighting is just a nice addition to it. And if missing for some text editors, this is probably not too bad.
@DanTup Is that something we can do using LSP (both highlighting two disjoint regions and providing multiple ranges that users can move between)?
I'm not sure if I completely understand exactly the flow. We can't do anything while the user is moving up and down items in the CodeAction list (we have no code executing there). When are are running code, we could highlight code in VS Code by using decorations (this was the suggestion when I asked for an API for highlighting code), but it'd have to be done in the extension (there's no LSP standard for this).
I wonder if an option for his would be to allow the user to just create a selection over the widgets to be extracted (before opening the lightbulb menu)?
The CodeAction text could be written to indicate this in some way ("Extract SizedBox + ColouredBox", "Extract SizedBox (+ 4 widgets) + ColouredBox").
I'm not sure if I completely understand exactly the flow.
I don't think we know the flow yet; that's what we're exploring.
IIRC, on the IntelliJ side, at least for 'Extract local variable' (I think that's what was being used), the flow is that the user selects the refactoring from the menu and then a menu pops up to allow the user to select the expression to be used as an initializer. Or in the case of this refactoring, possibly some description of which widgets are to be extracted.
I'm guessing that on the VS Code side the equivalent would be to have the refactoring return a list of ranges, use the prompt box to present the list, change the highlight range as the user arrows through the list, and then apply the selected set of edits.
On the IntelliJ side we have the ability to have a longer running dialog between the client and the server than what we're currently supporting in LSP, so we could delay computing the changes until we know which widgets are selected. But that's an implementation detail, not a UX design question.
I'm guessing that on the VS Code side the equivalent would be to have the refactoring return a list of ranges, use the prompt box to present the list, change the highlight range as the user arrows through the list, and then apply the selected set of edits.
Yep, I think we could do that (although it would be VS Code specific - or at least, require some custom client code and LSP requests which in theory other editors could implement similar to the other new refactor work).
Is your feature request related to a problem? Please describe. Often I need to extract a bigger piece of widget tree to a widget that exposes a single
child
property. This may not be entire subtree but some part of it e.g. fromA->B->C->D
I want to get toA->B'->D
. To achieve that I create a new widget withchild
property and copy and paste part of the tree that needs to wrap this child. For example starting with following:I end up with this structure:
Describe the solution you'd like I would like to have ability to wrap any widget with a single child wrapper widget similarly as I can extract part of subtree to a new widget.
Describe alternatives you've considered
Additional context
Copied from https://github.com/Dart-Code/Dart-Code/issues/4566 as requested by @johnpryan