Dart-code: Implement refactor: Move file

[DanTup]

Blocked on https://github.com/microsoft/vscode/issues/43768 being finalised.

Update: The VS Code API works, however despite supporting multiple files, it is only ever fired once-per-file https://github.com/microsoft/vscode/blob/926fc23f139e4d1250dba494c6e825e4816e62f6/src/vs/workbench/api/common/extHostFileSystemEventService.ts#L148 concurrently. This results in multiple MOVE_FILE calls to the analysis server, which currently only supports one refactor at a time (a new refactor cancels the previous).

The implement this reasonably (and without causing a potentially-large number of sequential requests to the server) we need to either have the server support multiple files, or do more work on the server (eg. via LSP).

[DanTup]

I don't think putting refactors on the lightbulb is a good idea, as they show up so frequently that there will just be bulbs everywhere always. I suggest these go on the context menu:

https://github.com/Microsoft/vscode/issues/9827

[DanTup]

Maybe this should be hooked into rename and move in the explorer?

https://github.com/Microsoft/vscode/issues/43768

[DanTup]

I've started work on adding this to the server, but it's not complete yet. See https://github.com/dart-lang/sdk/issues/33605.

[knopp]

No news about it? Renaming file in dart project is a bit annoying since it breaks all the imports.

[DanTup]

Sorry, the work on this somewhat stalled due to other priorities and not many people asking for this. I think the work in the server supports renaming files, just not folders. It might be possible to wire just that bit up - I'll try and take a look soon.

[bsutton]

Just wanted to add my voice to this.

Managing imports in dart/vscode is a pain.

Moving or renaming a file takes significant effort to re-do each of the affected imports.

[DanTup]

The work is mostly done here, but it requires a VS Code feature that is currently part of the provisional API. In order to ship this, we need it to be moved to the stable API.

The relevant issue is https://github.com/microsoft/vscode/issues/43768. I'll ping again to see if there's any update.

[shroudedcode]

@DanTup Maybe you're already aware, but the APIs you were talking about have been moved to stable and are available in the latest version of VS Code (1.41.1).

[DanTup]

Yep, I'd seen that, though I haven't had chance to fix up the work for it yet. I'll try to look at it soon though :-)

[fryette]

@DanTup do you have some estimations?

[bsutton]

@fryette

I wrote this as a stop gap:

https://github.com/bsutton/drtimport/blob/master/README.md

[carlowenig]

Any updates on this?

[DanTup]

Sorry, no progress yet. There's ongoing work to move to the LSP protocol, so it may make sense to do this as part of/after that (to avoid doing it twice). I'm not sure whether this behaviour is currently available in LSP though (so it may need some custom work).

[knopp]

Should be possible with LSP.

[DanTup]

That API allows the server to modify/delete/rename files, however the new API added in VS Code to support this was to tell the language server before files are renamed on the client so the server can compute the correct edits for the rename. I can't see anything in LSP that provides the same functionality (things are often added to LSP after being added to VS Code).

[knopp]

I see. That might indeed need custom work.

[bsutton]

This feature is in our top four friction points with the dart tooling for vscode.
Which for the record are:
this issue.
lack of auto completion of method arguments
minimal refactoring tools
copy/paste of required import statements when copying a piece of code.

[DanTup]

@bsutton thanks for the notes - it's worth putting a 👍 on this (and other) issues, as I do periodically review the list sorted by these.

As for the other things, I'm not sure exactly what the first two are specifically (but if there aren't existing issues, please do file some) but the third is currently blocked by VS Code (add a 👍 there too!) because it doesn't give us any API to know when the user copies/pastes (we need these in order to capture the imports required when you copy, and then insert them when you paste).

[DanTup]

I fixed up the existing work on this to use the updated VS Code API, but the API is not entirely compatible with the Dart analysis server. VS Code may send many rename requests at the same time if you drag multiple files but the Dart analysis server can only process one refactor request at a time (and only supports a single filename) so it's going to need some server work (and therefore may be easiest to do as part of the LSP migration).

[nhwilly]

If there were Dart support in VS 2019 Professional, I'd drop VS Code (which I love) for just this single feature of moving files around. In the early days of product development, when the structure is in flux, this is an enormous pain. How many thumbs up can I give this? 👍

Love VSCode, though. ❤

[DanTup]

If there was Dart support in VS, it would likely back on to the same language server, so likely need the same changes (or to sit in a loop sending lots of requests for each file and consolidating them).

I did start it once, but the VS APIs were infuriating to work with, then VS Code came along. VS does have (or is getting) LSP support, so once Dart-Code is fully migrated, it's likely that VS could be supported without too much effort (though it might not be something I'd take on!).

It's possible we could make it work here sooner by restricting it to only work for single files (eg. if you drag a folder or multiple files, it won't do anything - but a single file move/rename would update). It would be lost in the LSP transition until LSP supported it though (https://github.com/microsoft/language-server-protocol/issues/984).

[nhwilly]

Thanks for the quick response.

I'd settle for a one-file-at-a-time solution if it were available, but I don't drop my wishes on anyone doing enhancements for free. 🙂

If it happens, great, if not, no worries.

[DanTup]

I had another go at making this work, but I found a bug in VS Code when dragging multiple files - please 👍 this issue:

https://github.com/microsoft/vscode/issues/98309

In the meantime, I'll make it work for a single file (if you drag multiple, it'll probably only process the first).

[DanTup]

Ok, going to ship this behind a flag for vNext (dart.previewUpdateImportsOnRename):

It only works for single-files and no folders, but that's probably better than nothing. When the VS Code issue is fixed, I think we can support multiple files, though it may be slow for large numbers (due to lots of round trips to the server). Supporting multiple files quickly and/or folders will probably be best done in the server along with LSP once it gets the API.

I've opened some extra issues to track the other work:

• https://github.com/Dart-Code/Dart-Code/issues/2482 - remove preview flag
• https://github.com/Dart-Code/Dart-Code/issues/2483 - support multiple files

[acoutts]

Will this also work for imports which are using the absolute package: format?

Ex: package:my_app/foo/bar.dart

[DanTup]

It should do - though I did find some issues (described in https://github.com/dart-lang/sdk/issues/41889) that will need to be fixed - I think they're edge cases though.

I'll make a test build later and post the link here if anyone wants to try it out before the release and provide feedback(/find more bugs 😬).

[DanTup]

There's a beta build here if you'd like to try it out:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.11.0-beta.3

Download the .vsix file, then run Extensions: Install from VSIX from the VS Code command palette and select the file.

Then set "dart.previewUpdateImportsOnRename": true in your VS Code settings and restart.

If you have VS Code set to auto-update extensions, when this version is released stable, you'll automatically be updated, so you don't need to worry about uninstalling (unless something is broken - but please file issues if it is!).

[nhwilly]

Wow. OK!

[nhwilly]

You the man.

I generate my dto classes from a C# project and just send them straight to a folder in the Dart project. There are 40+ files now and it'll probably swell to 150.

Now I can move the whole folder at once except for the export file. This breaks everything of course. But then I move the export file and all the references are updated.

This is great!

Thank you SO much.

[DanTup]

@nhwilly great, glad to hear it'll help! I'll try to push on the remaining VS Code and analysis server issues when I can to handle the remaining cases. Thanks for nudging me to fix it up (there's no motivation like someone mentioning another editor 😄).

[bsutton]

I've manged to install the vsix.

I have two project.
1 is just a folder and the move works a treat :)
2 is a workspace and the move doesn't work.

I'm using the same settings file in both projects ( I copied it from the workspaces that doesn't work).

{
    "files.exclude": {
        "**/.classpath": true,
        "**/.project": true,
        "**/.settings": true,
        "**/.factorypath": true
    },
    "java.configuration.updateBuildConfiguration": "automatic",
    // "dart.debugExternalLibraries": true,
    // "dart.debugSdkLibraries": true


    "dart.previewUpdateImportsOnRename": true,
}

[DanTup]

@bsutton in a multi-root workspace, it wouldn't be read from each folders own .vscode/settings.json file as it's marked as a "window-scoped" setting (this means it can only have one value per VS Code window).

It should, however, be ready from the .code-workspace file in such a workspace. Is that what you tried? (You could also set it in your User Settings). If either of those don't work, please file an issue with an analysis server log captured during the drag, so we can see if the request made it to the server. (Also check Help -> Toggle Developer Tools to ensure there are no relevant errors in the console).

Thanks!

[bsutton]

This workspace actually only has one folder.

In the explorer tab I see:
(workspace)

• .vscode
  - - settings.json.

This settings file has the import option.

If I open the above noted folder directly the import management works.

[bsutton]

Sorry,
ignore my last comment I scanned over your comment a little too quickly.

I've now added the setting it the .code.workspace file and the import management works :)

[MarsGoatz]

Fantastic! Thank you for this extension!

[ericdallo]

@DanTup any idea to give LSP this support?
If dart-sdk could provide that via LSP we could use on other editors like lsp-dart

[DanTup]

@ericdallo currently LSP doesn't have the events required for this - there's an issue at https://github.com/microsoft/language-server-protocol/issues/984 to try and spec it. I think it should be relatively straight forward (just copying VS Code's API, and the waitFor can just be handled by waiting for the response from the server), though I haven't thought about it too much just yet.

[ericdallo]

It makes sense @DanTup, meanwhile do you think it worths implement this as a custom method on dart-sdk analysis like the other methods like closingLabels and outline?
I really miss that feature on lsp-dart :pensive:

[DanTup]

I'd prefer to avoid a custom method if we can get it in the spec fairly quickly, as a custom method would likely have to hang around for a long time (as editors may start depending on it).

I'll take a look at making a proposal for LSP to get feedback - I think it should be a fairly simple request.

[ericdallo]

I see, thank you @DanTup for the help on that :)