Skip to content

git: Open File command fix on clean tree - #60426

Merged
João Moreno (joaomoreno) merged 1 commit into
microsoft:masterfrom
ParkourKarthik:git-open-file
Mar 5, 2019
Merged

João Moreno (joaomoreno) merged 1 commit into
microsoft:masterfrom
ParkourKarthik:git-open-file

Conversation

@ParkourKarthik

Copy link
Copy Markdown
Contributor

Fixes #60174

@ParkourKarthik

Karthikayan (ParkourKarthik) commented Oct 10, 2018 •

Copy link
Copy Markdown
Contributor Author

João Moreno (@joaomoreno) I made a simple fix without making much change.
Thought of replacing the resources being loaded for uris, but ain't sure about the impact from other calls if any.
https://github.com/Microsoft/vscode/blob/354956f6382236000f2272da2e4a513d74dabcef/extensions/git/src/commands.ts#L602

For now, I've included the fix based on the reference from getSCMResource.
https://github.com/Microsoft/vscode/blob/354956f6382236000f2272da2e4a513d74dabcef/extensions/git/src/commands.ts#L2031

Review and let me know your thoughts

uris = resources.map(r => r.resourceUri);
}
else {
uris = [(window.activeTextEditor && window.activeTextEditor.document.uri)] as Uri[];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If window.activeEditor is undefined, this will cause uris to be an array with one element (undefined). Better use the ternary operator.

uris = (activeTextEditor ? [activeTextEditor.document.uri] : []) as Uri[];

Also you can move the activeTextEditor declaration above the uris declaration (close to the start of the openFile function), and reuse that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, for the late response. You're right about the check. But I doubt that this command will be invoked without an active editor. Because this command would not be accessible when there is no active editor. Let me know if I'm wrong.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually the window.activeTextEditor is a nullable field (of type TextEditor | undefined) hence using the existence check is required, otherwise there can be bugs in the future.

@joaomoreno

Copy link
Copy Markdown
Contributor

Thanks! 🍻

@joaomoreno
João Moreno (joaomoreno) merged commit 354956f into microsoft:master Mar 5, 2019
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

git GIT issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open File shortcut does not work if tree is clean

3 participants