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

Fix :tabe {file} only relative to current file (#1162) #2400

Merged

Conversation

arussellk
Copy link
Contributor

Fixes #1162

What this PR does / why we need it

This PR extends the logic for :tabe to better match user expectation. :tabe now handles the following situations:

  1. :tabe {file} when you have a workspace open will try to open {file} relative to the root of your first workspace folder
  2. :tabe {file} when you do not have a workspace will fall back to open {file} relative to your current document
  3. :tabe will open an untitled document
  4. :tabe {absolutePath} will open {absolutePath}

Which issue(s) this PR fixes

Fixes #1162

Special notes for your reviewer

  1. I was unable to use fs.mkdir to make a folder because I kept getting a EACCES: permission denied, mkdir error. Because of that, I did not add tests for when a user is in a workspace. I did add tests for the other cases that :tabe handles.
  2. I am now exporting testUtils.ts function createRandomFile for use in my tests.

VSCodeVim#1162

:tabe {file} will now search through the first folder in your workspace (if
available) to try to open {file}.

If you are not in a workspace, :tabe {file} will search relative
to your current file.
@TravisBuddy
Copy link

Travis tests have failed

Hey Russell Kennington,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

gulp forceprettier
[19:21:11] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[19:21:11] Starting 'forceprettier'...
[19:21:23] Finished 'forceprettier' after 12 s

@jpoon jpoon self-requested a review February 26, 2018 05:13
@@ -43,7 +43,7 @@ export class TabCommand extends node.CommandBase {
}
}

execute(): void {
async execute(): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that we should make this async. However, can you update the other return paths to also return a promise?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpoon I awaited everything I could with the following two exceptions:

  1. await vscode.commands.executeCommand('workbench.action.openEditorAtIndex1'); never resolves. I think it may be a bug with VSCode, so I should reproduce it in a small extension and open an issue on the VSCode repo.
  2. vscode.commands.executeCommand('activeEditorMove') needs to be updated to vscode.commands.executeCommand('moveActiveEditor'). It isn't quite a simple name replacement, so it should probably be done in a different PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #2401 for the refactoring.

@jpoon jpoon merged commit 0b6dec0 into VSCodeVim:master Feb 26, 2018
@jpoon
Copy link
Member

jpoon commented Feb 26, 2018

Thanks @arussellk

@arussellk arussellk deleted the fix/#1162-tabe-relative-to-current-file branch March 1, 2018 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

:tabe {file} is relative to current file
3 participants