Enable file actions in search results viewlet

[kisstkondoros]

Resolves #8594

[mention-bot]

@kisstkondoros, thanks for your PR! By analyzing the annotation information on this pull request, we identified @sandy081 and @egamma to be potential reviewers

[bpasero]

I am not so happy with all the dependencies from search => file, typically we try to keep the inter-part-dependencies to one file (a top level files.ts or search.ts) and do not reach into the more internal parts of it.

If this is about having an action to copy the path, I suggest to just contribute this one action from the search viewlet.

[bpasero]

I documented this in https://github.com/Microsoft/vscode/wiki/Code-Organization#workbench-parts now 👍

[kisstkondoros]

@bpasero Thanks for the explanation, and for writing it down somewhere!

If I got it right, the problem is about

import { keybindingForAction } from 'vs/workbench/parts/files/browser/fileActions';
import { FileStat } from 'vs/workbench/parts/files/common/explorerViewModel';

In that case I'm wondering what can i do about 'keybindingForAction'.
Since there is nothing like e.g. parts/common there is no place to put, well the common stuff.

I have a look at FileStat, perhaps it can be replaced with IFileStat...

I guess you didn't really liked

menuService.createMenu(MenuId.ExplorerContext, keybindingService)

either (in vs\workbench\parts\search\browser\searchResultsView.ts)

[bpasero]

@kisstkondoros do I understand your PR correctly that you want to contribute all file actions to the search viewlet or just the copy path action? because that one should not need the keybindingForAction?

[kisstkondoros]

@bpasero IMO it totally makes sense

[bpasero]

I do not fully agree, I would not expect file modification actions nor compare actions nor a way to copy a file in search results. For the purpose of solving #8594 I would introduce just one action to copy the path into the clipboard.

[sandy081]

I agree with @bpasero on above comment. But just showing one action might not look good. Showing Open specific actions makes sense to me.

Also, I think providing a context menu here might open up following questions:

• Expected to have the same in Problems view or other views where we have similar UI ?
• Expected to be consistent across views, similar context menus?
• Support for externally contributed context actions? If Markdown file is part of the result, then we might need preview action?

[bpasero]

Yes, whatever we do in search it should be contributable using the new menu extensions.

[kisstkondoros]

@bpasero I did reduced the set of commands to only 'copy path' but i have no idea how to do it in a clean way, so that none of the rules are broken (currently the copy path action has been duplicated).

@sandy081 You are right, it feels odd.

Since I don't know how to do it properly, I'm thinking about abandoning the pull request

[bpasero]

@kisstkondoros I think unfortunately you picked a time for doing this PR where we are in the middle of introducing a new concept for providing context menu actions (see https://github.com/Microsoft/vscode-docs/blob/vnext/release-notes/June_2016.md#menu-items-and-context-menu-entries). So, whatever we used to do in the workbench and editor to build the context menu is considered deprecated and old school and we intend to get those things converted to the new model.

As such, for the search viewlet we should also use the new way and I think @sandy081 and @jrieken are teaming up to get this going.

I will assign @sandy081 to be the owner of this PR as such so that he is aware of the code and the desire to have a "Copy Path" command. However, I think we should not use the "old way" for menus.

[sandy081]

My opinion was not to have just one entry in the context menu as it does not look good.

Since context menu entries are contributable, why not providing an extension? I am not sure how many users would want to have it by default.

[bpasero]

Interesting idea, do we have all the things in place to contribute to the search result context menu?

One issue I can see is that from an extension it might be tricky to get access to the clipboard.

[bpasero]

Btw I think we can have more entries in the context menu. Candidates are:

• Open to the Side
• Reveal in Finder / Explorer
• Copy Path

[sandy081]

No Search results context menu is not yet ready for contributable. This has to be kept in mind while implementing context menu for files in Search.

How about providing Search / Replace specific action also in the context menu, like, Remove, Replace, Replace all? Probably they are duplicates of the actions on the right but just a thought?

[bpasero]

Yes we typically do not repeat the actions that are showing as primary actions on the element within the context menu (which only has secondary actions). But if there are search/replace specific actions not showing up as primary, we can add them too.