Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement refactor: Move file #548

Closed
DanTup opened this issue Feb 8, 2018 · 47 comments · Fixed by #1740
Closed

Implement refactor: Move file #548

DanTup opened this issue Feb 8, 2018 · 47 comments · Fixed by #1740
Labels
in editor Relates to code editing or language features is enhancement
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Feb 8, 2018

Blocked on microsoft/vscode#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 DanTup added is enhancement in editor Relates to code editing or language features labels Feb 8, 2018
@DanTup DanTup added this to the On Deck milestone Feb 8, 2018
@DanTup DanTup added the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Feb 12, 2018
@DanTup
Copy link
Member Author

DanTup commented Feb 12, 2018

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:

microsoft/vscode#9827

@DanTup DanTup removed the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Feb 21, 2018
@DanTup
Copy link
Member Author

DanTup commented Feb 21, 2018

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

microsoft/vscode#43768

@DanTup DanTup added the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Feb 21, 2018
@DanTup DanTup modified the milestones: On Deck, Backlog Feb 21, 2018
@DanTup DanTup modified the milestones: Backlog, On Deck Oct 30, 2018
@DanTup DanTup added blocked on dart / flutter Requires a change in Dart or Flutter to progress and removed blocked on vs code / lsp / dap Requires a change in VS Code to progress labels Oct 30, 2018
@DanTup
Copy link
Member Author

DanTup commented Oct 30, 2018

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

@knopp
Copy link

knopp commented May 15, 2019

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

@DanTup
Copy link
Member Author

DanTup commented May 15, 2019

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.

@DanTup DanTup removed the blocked on dart / flutter Requires a change in Dart or Flutter to progress label May 23, 2019
@DanTup DanTup modified the milestones: On Deck, v3.2.0 May 23, 2019
@DanTup DanTup added the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Jun 6, 2019
@DanTup DanTup modified the milestones: v3.2.0, On Deck Jun 6, 2019
@bsutton
Copy link

bsutton commented Aug 20, 2019

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
Copy link
Member Author

DanTup commented Aug 20, 2019

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 microsoft/vscode#43768. I'll ping again to see if there's any update.

@shroudedcode
Copy link

@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
Copy link
Member Author

DanTup commented May 21, 2020

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

rename_preview

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:

@DanTup DanTup modified the milestones: Backlog, v3.11.0 May 21, 2020
@acoutts
Copy link

acoutts commented May 21, 2020

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

Ex: package:my_app/foo/bar.dart

@DanTup
Copy link
Member Author

DanTup commented May 21, 2020

It should do - though I did find some issues (described in dart-lang/sdk#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
Copy link
Member Author

DanTup commented May 21, 2020

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
Copy link

nhwilly commented May 21, 2020

Wow. OK!

@nhwilly
Copy link

nhwilly commented May 21, 2020

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
Copy link
Member Author

DanTup commented May 21, 2020

@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
Copy link

bsutton commented May 22, 2020

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
Copy link
Member Author

DanTup commented May 22, 2020

@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
Copy link

bsutton commented May 25, 2020

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
Copy link

bsutton commented May 25, 2020

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
Copy link

Fantastic! Thank you for this extension!

@ericdallo
Copy link

@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
Copy link
Member Author

DanTup commented Aug 7, 2020

@ericdallo currently LSP doesn't have the events required for this - there's an issue at microsoft/language-server-protocol#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
Copy link

ericdallo commented Aug 7, 2020

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 😔

@DanTup
Copy link
Member Author

DanTup commented Aug 12, 2020

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
Copy link

ericdallo commented Aug 12, 2020

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

@om-ha
Copy link

om-ha commented Oct 2, 2021

Is it possible to move a file (and fix imports) programmatically from shell/bash CLI?

UseCase: You have some auto-generated file *.g.dart that gets saved somewhere you don't like, you want to move this auto-generated file to another path within your /lib directory. However within this file, some imports are relative. Moving the file from VS Code UI auto-fixes the imports (option called updateImportsOnRename from settings.json, official docs here).

Is there a way to do this programmatically?

@DanTup
Copy link
Member Author

DanTup commented Oct 6, 2021

@om-ha unfortunately not. For the renames to work, the analysis server needs to be told about them before they happen. You could perhaps automate it within a VS Code extension that does the move (which would trigger the APIs that talk to the language server), but that's not very ad-hoc.

@bsutton
Copy link

bsutton commented Oct 6, 2021

@om-ha
This may help:
https://pub.dev/packages/drtimport

It's only 'lightly' maintained.

@cedvdb
Copy link

cedvdb commented Dec 1, 2021

This seems to be buggy lately, in multiple projects. Has anyone have had imports not updated ?

=> Dart SDK version: 2.16.0-10.0.dev

@DanTup
Copy link
Member Author

DanTup commented Dec 1, 2021

@cedvdb I haven't seen any reports of this. Can you reliably reproduce it? It's possible that if the refactor takes too long VS Code times it out and just proceeds with the rename without waiting for the edits, but I'm not certain.

A log file could be useful (captured with Dart: Capture Analysis Server Logs) though note it'll include parts of your source code (so don't share them from any sensitive projects). Please file a new issue with the logs or more info. Thanks!

@tanphat110699
Copy link

tanphat110699 commented Apr 6, 2022

@DanTup Sir, can this feature support delete/rename folder? This will be awesome! Thank you so much.

@DanTup
Copy link
Member Author

DanTup commented Apr 6, 2022

@tanphat110699 there's an open issue about that here:

#2483

A lot of the work is done (for single-folder renames), but I'm waiting for a VS Code LSP client release (which I expect in the next month or two) which has support for proper cancellations. Without that, if you rename a large folder and it takes a long time to compute the edits for updating imports, you are unable to cancel it and that's not a very good experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features is enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.