Cell context menu should use NotebookCell as context like the cell toolbar

[rebornix]

The cell context menu in notebook editor is using UriComponents as context, but the cell toolbar is using NotebookCell. Since we populate commands from cell toolbar to the cell context menu, those commands won't work in cell context menu. We should have them aligned.

[roblourens]

I don't think we have any way to customize this, the editor controls the context and all editor context menu actions have the same context

vscode/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts

Line 162 in 9fa8f02

const groups = menu.getActions({ arg: model.uri });

We could handle this for builtin commands, but for commands from extensions, it would still be better to give the same context as the cell toolbar. Maybe we could register a wrapper action for every command in the cell toolbar that translates the context and executes the real command? Or we have to plumb a way for the editor to let us change the context of some menu items or something like that which seems awkward. Any other ideas @rebornix @jrieken?

[jrieken]

I think using NotebookCell for the editor/context menu is undesirable and unexpected. Today things "just work" - meaning a normal editor command also works in notebooks because it is just text editors/documents. The contract for the context menu shouldn't not be changed. This similar to how the editor context menu for output channels isn't the output channel object but also its uri.

[roblourens]

I'm not saying that all editor commands should have a different context- only commands that end up in the "Notebook Cell" submenu automatically as a result of being contributed to the cell title menu:

vscode/src/vs/workbench/contrib/notebook/browser/controller/coreActions.ts

Line 426 in 9fa8f02

submenu: MenuId.NotebookCellTitle,

The result would be that a notebook command gets the same context whether it was invoked from the cell title toolbar or from the cell context menu. Other editor commands wouldn't be affected.

We can also say that commands will get a different context depending on which menu they are invoked from, but it seems nicer to be consistent especially if extensions will contribute to the toolbar and not test the command in the context menu, then they don't work there (as in microsoft/vscode-jupyter#7751)

[jrieken]

Oh, now I get it. Unsure what/how to do it. Why do we have this menu at all? It seems weird to duplicate a title menu into the context menu

[rebornix]

I can see the value of having notebook cell actions in the editor context menu as users would always get those actions no matter if they right click on cell container, or cell editor. My original issue was about aligning the context for Cell container context menu (right click on cell container/chrome), which is different from the cell editor context menu.

I'm not convinced that we want to change how we populate context for editor context menu so thinking about this again, I think it's fine that cell commands' context is NotebookCell | Uri | undefined. Leaving to @roblourens for the final call.

[roblourens]

My original issue was about aligning the context for Cell container context menu (right click on cell container/chrome), which is different from the cell editor context menu.

Oh, I didn't even pick up on that.

I'm not convinced that we want to change how we populate context for editor context menu so thinking about this again, I think it's fine that cell commands' context is NotebookCell | Uri | undefined.

I think that's ok, if only because we don't have a good solution to customizing the context for editor cell commands. Then for this issue I'll just fix commands to work with a Uri context. These menu contexts aren't really documented for extensions though, right? It's a bit tricky to work it out by experimenting.

It seems weird to duplicate a title menu into the context menu

You can even hide the toolbar 😭