Adopt vscode.open or vscode.diff in built-in extensions

[bpasero]

With #85636 landed, it is now recommended that an extension sets the TreeItem command to vscode.open or vscode.diff if the resource is know upfront.

Example from ref view:

https://github.com/microsoft/vscode-references-view/blob/730800271aa5ef52b1c49608d68f14ee585f7821/src/calls/model.ts#L176-L186

List of extensions with custom views we ship:

• JavaScript Debugger @connor4312
• JavaScript Perf View @connor4312
• NPM @alexr00
• References View @jrieken already did that to my knowledge

Git support is tracked in #110397 and timeline in #110496

//cc @isidorn @weinand not sure if debug has custom views where we should support this as well?

[isidorn]

Debug had loaded scripts view, but it is now in the vscode core. So we should be covered there.

[connor4312]

We have some views in debug land but don't open sources from them.

[alexr00]

NPM involves opening package.json files and finding specific scripts in them. Adopting vscode.open there should wait until #110498

[bpasero]

@alexr00 can you clarify? at the time I have expanded the NPM tree to see scripts, we should have already resolved everything, no?

[alexr00]

The view uses fetchTasks to populate the tree view, which gives no information about where in the package.json file the task is coming from. The code can always be refactored to not do this, but it isn't just a simple adoption then.

[alexr00]

It probably should be refactored that way since it will be more efficient then.

[bpasero]

After a quick check, I think the only blocker is this code:

vscode/extensions/npm/src/npmView.ts

Line 198 in 7b5849b

let document: TextDocument = await workspace.openTextDocument(uri);

If the position was known upfront, this could be changed from:

vscode/extensions/npm/src/npmView.ts

Lines 82 to 86 in 7b5849b

	'open': {
	title: 'Edit Script',
	command: 'npm.openScript',
	arguments: [this]
	},

to something like:

'open': {
	title: 'Edit Script',
	command: 'vscode.open',
	arguments: [uri, selection]
}

[alexr00]

That is correct. I don't want to go and open every package.json multiple times up front, so the extension will need to be refactored to determine the line number up front.

[alexr00]

@connor4312 and @jrieken FYI I have made a test plan item for NPM scripts, but we will still need test items for the other extensions.

[jrieken]

I have a verificated-needed item for this

[bpasero]

I had this for my #111417 already spelled out, but having individual issues for each extension makes more sense to me:

Maybe I leave my thing in for SCM at least (@joaomoreno fyi)

[joaomoreno]

@bpasero I have this for Git/SCM: #111519

[bpasero]

Ok good then I will simply remove that part from my test plan item, thanks for covering it in other issues 👍