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

editor/lineNumber/context proposed menu #175945

Closed
joyceerhl opened this issue Mar 2, 2023 · 17 comments · Fixed by #179168
Closed

editor/lineNumber/context proposed menu #175945

joyceerhl opened this issue Mar 2, 2023 · 17 comments · Fixed by #179168
Assignees
Labels
Milestone

Comments

@joyceerhl
Copy link
Collaborator

Re: #175676

Currently clicking on a line number in the editor selects the whole line, but right clicking on it does not show a context menu. To mirror the behavior that GitHub offers for generating deeplinks to a line in a file, we'd like to introduce a context menu attached to line numbers in an editor, like so:

image

This new proposed menu should integrate well with the existing menus that appear in the editor gutter. We currently have the breakpoint editor contribution, which shows a menu of breakpoint actions on a right click in the editor gutter. Additionally, when test decorations are rendered, we show both breakpoint actions as well as testing actions. There is already a menu ID for the gutter when a testing decoration is rendered, testing/item/gutter, which accepts contributions from extensions.

This is the behavior I'm currently looking to implement:

  • If line numbers are enabled:
    • Context menu for editor/lineNumber/context should contain actions for:
      • Test
      • Debug
      • Copy
  • If line numbers are disabled:
    • Context menu for testing/item/gutter should contain actions for:
      • Test
      • Debug
    • Reasoning: it doesn't seem like a natural interaction to want Share actions from the gutter if line numbers are turned off

In other words, I plan to contribute Copy __ Link actions to both editor/lineNumber/context and testing/item/gutter, and the when clause for the Copy contributions to the latter menu would be "when": "config.editor.lineNumbers != off".

(It's a little odd to have copy actions contributed to testing/item/gutter so I wonder if we could rename to editor/gutter/context and pull in any contributions to testing/item/gutter for backwards compatibility, cc @connor4312)

@connor4312
Copy link
Member

pull in any contributions to testing/item/gutter for backwards compatibility

I think this is the way to go 👍

@joyceerhl
Copy link
Collaborator Author

Feedback from API sync:

  • Overall idea seems fine
  • Unsure whether this should be named editor/lineNumber/context or editor/lineNumbers/context
  • Context object is currently { uri: vscode.Uri; lineNumber: number }, perhaps it should be { uri: vscode.Uri, position: vscode.Position }
  • Unsure if each contrib's link implementation should hide itself when line numbers are disabled, arguably line numbers are a visual thing and I still want the copy actions. May be confusing, waiting for additional feedback

@alefragnani
Copy link

Hi,

First of all, thank you for starting this API proposal. It will be really useful for some extensions 🤝

I was able to play with it, and TBH, seems fairly simple to use. But, unless I'm missing something, the when clause seems limited for this API.

I mean, similar to how Breakpoints works, I would like to only display Add Bookmark or Remove Bookmark menus if the current line has or not bookmarks. It doesn't seems possible today.

I see two different approaches.

  • An event to be fired prior to the context menu to be shown. So, in this event, I could call setContext with the lineNumber that the event should pass as parameter, evaluating if the line has bookmarks. And it would be used as regular when clauses
            "editor/lineNumber/context": [
                {
                    "command": "_bookmarks.addAtLine",
                    "when": "!bookmarks.hasBookmarkAtLine"
                },
                {
                    "command": "_bookmarks.removeAtLine",
                    "when": "bookmarks.hasBookmarkAtLine"
                }
            ],
  • The API also provides a lineNumber context. So, I could use in and not in conditional operators, evaluating with the list of bookmarks that the current file has, like:
            "editor/lineNumber/context": [
                {
                    "command": "_bookmarks.addAtLine",
                    "when": "lineNumber not in bookmarks.linesWithBookmarks"
                },
                {
                    "command": "_bookmarks.removeAtLine",
                    "when": "lineNumber in bookmarks.linesWithBookmarks"
                }
            ],

Am I missing something here, some additional documentation for this proposal that I should look for, or is this scenario still not covered?

Thank you

@eamodio
Copy link
Contributor

eamodio commented Mar 9, 2023

Yeah I would assume you'd need to use the in conditionals. This does kind of beg for another type of when clause to actually be an indexer. So that you could use the lineNumber as an index into an array or object/map in another context value. That way you could access the value of that lookup to make further conditionals without having to create many other mapping in calls.

Maybe something that would be more possible with the new when clause parser.

@eamodio
Copy link
Contributor

eamodio commented Mar 9, 2023

What value would position provide over just the number? Wouldn't the column value always be 0? Also I think being a position would make the usage of in impossible (since I don't think you could access the line property)

@alefragnani
Copy link

alefragnani commented Mar 9, 2023

Right now, it only provides lineNumber and uri. Something like:

    interface EditorLineNumberContextParams {
        lineNumber: number,
        uri: Uri
    }

That's the reason why I suggested the in / not in conditional operators as an alternative

@alefragnani
Copy link

In fact, I didn't remember there was in and not in conditional operators, up until the moment I noticed it wouldn't be possible to do what I need without something else. Then I navigated to the API documentation, in order to look for alternatives, and there it was. A good alternative, but that also requires additional info from the API.

@eamodio
Copy link
Contributor

eamodio commented Mar 9, 2023

@alefragnani what does it need to work with the in operator? Is lineNumber not provided to the when context?

@alefragnani
Copy link

Not that I'm aware of.

Maybe it is called something else, but a quick look at the issues/PRs but didn't find anything that could confirm.

@eamodio
Copy link
Contributor

eamodio commented Mar 9, 2023

Ah I see.

@joyceerhl I think you need to do something like (untested):

	public show(e: IEditorMouseEvent) {
		// on macOS ctrl+click is interpreted as right click
		if (!e.event.rightButton && !(isMacintosh && e.event.leftButton && e.event.ctrlKey)) {
			return;
		}

		const model = this.editor.getModel();
		if (!e.target.position || !model || e.target.type !== MouseTargetType.GUTTER_LINE_NUMBERS && e.target.type !== MouseTargetType.GUTTER_GLYPH_MARGIN) {
			return;
		}

		const anchor = { x: e.event.posx, y: e.event.posy };
		const lineNumber = e.target.position.lineNumber;

		const contextKeyService = this.contextKeyService.createOverlay([
			['line', lineNumber],
		]);

		const menu = this.menuService.createMenu(MenuId.EditorLineNumberContext, contextKeyService);

		const actions: IAction[][] = [];

		this.instantiationService.invokeFunction(accessor => {
			for (const generator of GutterActionsRegistry.getGutterActionsGenerators()) {
				const collectedActions: IAction[] = [];
				generator({ lineNumber, editor: this.editor, accessor }, { push: (action: IAction) => collectedActions.push(action) });
				actions.push(collectedActions);
			}

			const menuActions = menu.getActions({ arg: { lineNumber, uri: model.uri }, shouldForwardArgs: true });
			actions.push(...menuActions.map(a => a[1]));

			this.contextMenuService.showContextMenu({
				getAnchor: () => anchor,
				getActions: () => Separator.join(...actions),
				menuActionOptions: { shouldForwardArgs: true },
				getActionsContext: () => ({ lineNumber, uri: model.uri }),
				onHide: () => menu.dispose(),
			});
		});
	}

Here: https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/codeEditor/browser/editorLineNumberMenu.ts#L64

Probably would also want to add other resource* context keys similar to what gets set on the editor content menus:
https://code.visualstudio.com/api/references/when-clause-contexts#available-contexts
Probably via:

export class ResourceContextKey {

@joyceerhl
Copy link
Collaborator Author

It makes sense to make the line number available via a context overlay. I've opened a PR to add an editorLineNumber context key and tested that it works to enable/disable actions in this context menu for a test extension: #176659

@eamodio in my testing it seems like the other resource* context keys are already set because of the active editor, so I'm not sure explicitly setting that for the context menu is necessary--please correct me if I'm mistaken.

@alefragnani
Copy link

alefragnani commented Mar 13, 2023

Hi @joyceerhl,

Just to let you know, the new editorLineNumber context key is working perfectly.

line without bookmark line with bookmark
"when": "editorLineNumber not in bookmarks.linesWithBookmarks" "when": "editorLineNumber in bookmarks.linesWithBookmarks"
Screenshot 2023-03-13 at 13 12 06 Screenshot 2023-03-13 at 13 12 15

Thank you!

@AffluentOwl
Copy link

I had a few ideas for gutter annotations, and am wondering if all of the following are now possible after this change. I think #7 and #8 may be of particular interest to @joyceerhl

  1. Left Clickable Annotations - Can open something like a diff view on left click
  2. Right Clickable Annotation - Can open a context menu
  3. Hoverable Annotation - HTML tooltip fully controlled by an extension (hyperlinks, clickable buttons, images/graphs, etc?)
  4. Zero-margin/padding annotations - An annotation can fully consume its assigned gutter space, allowing things like git diff / source control annotations which draw contiguous green lines without gaps for sections of code.
  5. Exclusive Gutter Annotation Space - Allows gutter annotations from the same extension to be guaranteed to be displayed above and below each other. Ex: git diff, you want to guarantee the green lines are contiguous vertical lines across many lines of code. The downside is if there are 30 possible gutter annotations, of width X, this requires 30 * X width.
  6. Shareable Gutter Annotation Space - Some gutter annotations can be share space and rarely appear (breakpoints, run test, hot line), for these annotations they can all stack together in the same gutter space. So if there are 30 possible optional annotations, but only 3 apply to this line, it only requires 3 * X width.
  7. Interacting with Contiguous Ranges of Lines - Allow selecting a range of lines, and passing this as context to the extension. For example, you want to link to a range of code / snippet to show your coworker
  8. Interacting with Discontiguous Ranges of Lines - Allow selecting multiple ranges of lines, and passing this as context to the extension. Then share and link this snippet to your coworker.

Some of the numerous use cases by extensions to keep in mind when thinking through this feature:

Diffing

Visual - Show contiguous green / red / yellow lines for where code matches / doesn't match / partially matches.

Use Case - Extend VS diffing to allow diffing against arbitrary changes in the commit history, or between two arbitrary files the user is working on.

Left Click - Opens side by side diff or inline diff view

Right Click - Context menu to allow changing revision to diff against, or arbitrary file to diff against.

Hover - Revision or arbitrary file being diffed against

Performance Metrics

Visual - Heatmap using a gradient from Red to Blue showing CPU time or Wall time spent on each line of code. Alternative visual - Just a 🔥 glyph on lines above some threshold of resource usage.

Use Case - Allows devs to identify expensive lines, to either improve or be cautious about changing or adding to.

Left Click - Opens website with a flame chart of measured performance of that line of code. Alternative - Opens the flame chart in a pane owned by the extension.

Right Click - Context menu to change/drilldown/slice the backing data source. For example, which benchmarks triggered the annotations. Or for which services/users is this considered a hot line versus others it is not.

Hover - Shows a panel with the callstack that contributed most to the performance, with hyperlinks that will open those code locations in vs code. Also shows the backing data source.

Linters

You can imagine numerous different linters running (syntax error, style enforcement, common typos, banned words/regex, etc).

Diagnostics

Next to logging lines allows seeing a sampling of the values that were output at that line, along with drilling down and slicing those values.

Bookmarks

Unit Testing - Run/Debug test

@joyceerhl joyceerhl removed this from the March 2023 milestone Mar 20, 2023
@joyceerhl
Copy link
Collaborator Author

Feedback from API sync:

  • This is good to finalize in April
  • Maybe we want to also expose editorLineNumber in the editor context menu (versus the editor line number context menu) in future, but that can be done independently of this proposal

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Apr 4, 2023
@QuocTrung76
Copy link

Sorry, I do not clearly understand the current status of editor/lineNumber/context, can I use it for contributions point/menu, I downloaded vscode.proposed.contribEditorLineNumberMenu.d.ts but it is empty.

@gjsjohnmurray
Copy link
Contributor

@QuocTrung76 you should be able to reference the new menu in the package.json of your extension provided it specifies 1.78 as its minimum VS Code version. That also means you will have to use Insiders until 1.78 ships as the standard release (aka Stable), which is due by the end of next week.

@jantimon
Copy link

jantimon commented May 16, 2023

Works great in 1.78.2 👍

@github-actions github-actions bot locked and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants