Skip to content

Wrap with column missing in flutter IDEs #2492

Closed

Description

@ryaninja

I've tried this in both Android Studio and VSCode, but the context menu to wrap in column simply doesn't appear for me. I'm trying to follow a layout tutorial, but I get this error at pretty much the first hurdle. Has this feature been removed/replaced? Anyone got any ideas?

Here's what the guide looks like:
https://i.stack.imgur.com/fpKV2.jpg

And here's what I see:
https://i.stack.imgur.com/yZu5M.jpg

Activity

  1. ryaninja commented on May 25, 2020

    @ryaninja
    Author

    Ok, so it turns out you need to right-click and not highlight the widget and then right click. Wrap in column showed as soon as I did this.

  2. DanTup commented on May 25, 2020

    @DanTup
    Member

    @bwilkerson @scheglov does this seem like a bug? When invoking the assist menu one character from the end of a widget class, Wrap in Row/Column show up. However if the cursor is after the last character, they do not (however the others do). I can't think of a reason for this (nor can I see an obvious reason from the code why this happens):

    Screenshot 2020-05-25 at 15 40 43

    Screenshot 2020-05-25 at 15 40 51

  3. bwilkerson commented on May 25, 2020

    @bwilkerson

    I can't think of any valid reason for the behavior being different between these two selection locations, so I'd consider this to be a bug.

    Note that "Wrap with Row" is also missing. Both it and "Wrap with Column" are implemented in a single method. That method uses SelectionAnalyzer to convert a selection range to a set of AST nodes, and my guess (though I haven't dug into it enough to know for sure) is that that class selects an AST node that the method in question doesn't handle correctly, and that that's the source of the bug.

  4. DanTup commented on May 26, 2020

    @DanTup
    Member

    Ok, took a quick look at this. When the cursor is inside the name of the widget, analyzer.coveringNode from the SelectionAnalzyer correctly identifies the widget expression. However, when it's like this:

    WidgetFoo^('test')
    

    The selection analyzer picks up the argument list ('test') as the innermost covering node (since it starts at that offset), and flutter.identifyWidgetExpression exits early (without walking up parents) when it finds argument lists:

    https://github.com/dart-lang/sdk/blob/93313eb2449099e20ade80d4760f76a325a4e176/pkg/analysis_server/lib/src/utilities/flutter.dart#L325-L327

    Removing node is ArgumentList from that condition causes this case to work, however it breaks some other tests (for example test_identifyWidgetExpression_node_instanceCreation which is explicitly checking for this behaviour).

    I'm not sure what a good fix is. If feels like the cursor being to the left of a paren in an argument list maybe should not be treated as being in the argument list, but that also feels like a weird exception.

  5. reopened this on May 26, 2020
  6. added this to the v3.12.0 milestone on May 26, 2020
  7. bwilkerson commented on May 26, 2020

    @bwilkerson

    A possible short-term work around would be to check in the assists to see whether the covering node is an argument list whose parent is a widget expression. If we add a comment explaining why we're doing it, then we'd have some context in case we decide to change SelectionAnalyzer in the future.

    @scheglov I'm interested in your thoughts on this.

  8. DanTup commented on Jun 11, 2020

    @DanTup
    Member

    @scheglov do you have any additional thoughts on this? If not, I might have a go at doing what @bwilkerson suggested above. Thanks!

  9. bwilkerson commented on Jun 11, 2020

    @bwilkerson

    A note on implementation.

    In order to better support being able to apply multiple fixes simultaneously, we changed the way we're implementing both fixes and assists. They now use instances of a subclass of CorrectionProducer to build the actual edits. The fixes have all been converted at this point, but most of the assists have not been.

    Prior to these changes, fixes and assists each had a slightly different way of computing the node used as the starting point for creating the edits. Usually they produced the same node, but in a few cases they differed. In the new architecture they are consistent. That has meant that sometimes when converting a fix or assist we've had to update the code slightly to adjust for the change in starting node.

    So, if you're interested in looking into this, it would be good if you first converted the existing assist to use the CorrectionProducer pattern. It's possible that doing so would impact the way the bug needs to be fixed, and it would be nice not to have to do the conversion after the fix is applied. That said, I won't say to no to getting the bug fixed without the extra work!

  10. scheglov commented on Jun 11, 2020

    @scheglov

    It is mostly a matter of covering reasonable cases.
    It is reasonable to propose Wrap in this case Widget^(...).
    But we might have to do differently for Widget(..., child^).

  11. modified the milestones: v3.12.0, On Deck on Jun 22, 2020
  12. DanTup commented on Jun 25, 2020

    @DanTup
    Member

    So, if you're interested in looking into this, it would be good if you first converted the existing assist to use the CorrectionProducer pattern.

    Looks like you beat me to this :-)

    It is reasonable to propose Wrap in this case Widget^(...).
    But we might have to do differently for Widget(..., child^).

    I think we could specifically handle the case where the caret is at exactly the offset of the argument list (eg. the caret is to the left of the open paren) - coveringNode is ArgumentList && coveringNode.offset == selectionOffset? I've opened a change that does that here:

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

  13. removed this from the On Deck milestone on Jun 25, 2020
  14. 3 remaining items

  15. removed this from the v3.13.0 milestone on Jul 28, 2020
  16. mich040m commented on Feb 23, 2022

    @mich040m

    Still happening as described here

    @bwilkerson @scheglov does this seem like a bug? When invoking the assist menu one character from the end of a widget class, Wrap in Row/Column show up. However if the cursor is after the last character, they do not (however the others do). I can't think of a reason for this (nor can I see an obvious reason from the code why this happens):

    Screenshot 2020-05-25 at 15 40 43 Screenshot 2020-05-25 at 15 40 51
  17. bwilkerson commented on Feb 23, 2022

    @bwilkerson

    Yes, that looks like a bug. The question is: does the version of the Flutter SDK that you're using include the fix. It sometimes takes a long time for a fix to make it into a build. Unfortunately, I can't remember how to determine that information, even if I knew which version of the Flutter SDK you're using.

  18. DanTup commented on Feb 24, 2022

    @DanTup
    Member

    I can reproduce this, but it's slightly different to the original report/fix - in that case the cursor was at the end of ModelBinding (the highlight is just VS code highlighting the references) and that works. However if you select the widget name, Wrap with Column is still missing.

    # present
    "range":{"start":{"line":54,"character":11},"end":{"line":54,"character":11}}
    # present
    "range":{"start":{"line":54,"character":23},"end":{"line":54,"character":23}}
    # missing
    "range":{"start":{"line":54,"character":11},"end":{"line":54,"character":23}}
    
  19. reopened this on Feb 24, 2022
  20. added this to the v3.36.0 milestone on Feb 24, 2022
  21. DanTup commented on Feb 24, 2022

    @DanTup
    Member

    With dart-lang/sdk@8480657, these actions will also work if the whole widget name is selected. The fix is in the SDK so will ship with a future Flutter SDK release (rather than the Dart extension for VS Code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    in editorRelates to code editing or language featuresin lsp/analysis serverSomething to be fixed in the Dart analysis serveris bug

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions