-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
[typescript] Better paths matching for move to existing file quickpick #181231
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this. Looks like a nice addition but a few things look into before this can be merged
const filename = Utils.basename(uri); | ||
|
||
let description; | ||
const filterQuery = ['./', '../'].find(str => quickPick.value.startsWith(str)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On windows, paths will use \
instead. If possible though, it would be nice to support letting users type either ../
or ..\
on windows to access parent directories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course it is possible, not a big deal. Just didn't add support for it in the first place as never had to use \
in VS Code
const quickPick = vscode.window.createQuickPick(); | ||
const body = response.body; | ||
const updateItems = () => { | ||
const destinationItems = body.files.map((file): DestinationItem | undefined => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be optimized to not compute anything if the user has not typed ./
or ../
to start the path
extensions/typescript-language-features/src/languageFeatures/refactor.ts
Outdated
Show resolved
Hide resolved
extensions/typescript-language-features/src/languageFeatures/refactor.ts
Show resolved
Hide resolved
use coalesce
By the way last time I tried it in insiders, I did see move to file code action (TS server didn't see interactive arg). Does it work on your end? Update: I see it also works in latest insiders, my bad |
It's an improvement for #178535.
Moving statements to a sibling file is pretty common. In my case, I have nested file structure (5-6 levels deep) and it's hard to do this with current quickpick implementation, so I added simple filtering for
./
and../
values. Now it can be done much faster than with a native file picker.Summarized:
and added
matchOnDescription
showQuickPick
tocreateQuickPick
API to implement relative path logic