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

Support right click context for viewer nodes #208

Merged
merged 11 commits into from
Dec 26, 2019

Conversation

Vigilans
Copy link
Member

@Vigilans Vigilans commented Nov 19, 2019

It contributes menus such as Reveal in Explorer, Copy Path for #207

Based on:

Demo:
image

package.nls.json Outdated Show resolved Hide resolved
@@ -83,6 +83,21 @@
"dark": "images/dark/icon-link.svg",
"light": "images/light/icon-link.svg"
}
},
{
"command": "java.view.package.revealFileInOS",
Copy link
Contributor

Choose a reason for hiding this comment

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

revealFileInOS -> revealInExplorer

Copy link
Member Author

@Vigilans Vigilans Nov 28, 2019

Choose a reason for hiding this comment

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

revealInExplorer is not appropriate since it is related to two built-in commands in VS Code: revealInExplorer and git.revealInExplorer - They all refer to the meaning of "Reveal the file node in VS Code File Explorer", not the OS explorer.

In contrast, revealFileInOS is a built-in command of VS Code to open a file in OS explorer, and our command just forwards the dependency node's uri to it, so this naming would be fine.

Copy link
Contributor

@testforstephen testforstephen Nov 28, 2019

Choose a reason for hiding this comment

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

so why not to mount the VS Code builtin command to the dependency explorer directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be best that we could mount the builtin command directly.
But the builtin command accepts a Uri argument, where context entry specified in package.json only allows for passing a ExplorerNode to the command binding.

(The TreeItem.command property can indeed specify the argument to be passed, but this is not the case with view/item/context entry)

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems VS Code provides different names in different OSes, could you check whether we need keep consistent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for I didn't notice this comment...

As this file shows, VS Code does provide different names here:

  • Windows: Reveal In Explorer
  • Mac OS: Reveal In Finder
  • Linux: Open Containing Folder

And the Chinese translation uses that localization:

"vs/workbench/contrib/files/electron-browser/fileActions.contribution": {
    "revealInWindows": "在资源管理器中显示",
    "revealInMac": "在 Finder 中显示",
    "openContainer": "打开所在的文件夹",
    "filesCategory": "文件"
},

Currently I have no knowledge of how to utilize the localization here according to OS...

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently i'm OK to keep the same one, if there is user complaint, then fix it.
A workaround is add three difference commands, and use when clause to enable the correct one.

@testforstephen testforstephen merged commit b87c7b5 into master Dec 26, 2019
@jdneo jdneo deleted the vigilans/right-click-context branch March 16, 2020 06:44
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.

2 participants