RFC: Remove (or move, or improve) Register-EditorCommand, $psEditor etc.

[andyleejordan]

Prerequisites

• I have written a descriptive issue title.
• I have searched all issues to ensure it has not already been reported.

Summary

I have just discovered (after maintaining this extension for the better part of the year) this entire interface around registration of editor commands. It's somewhat documented, but it doesn't seem to completely work.

I found this as I was going through our package's contributed commands, specifically one of the entry points to it: InvokeRegisteredEditorCommand. In #2225 this command was supposed to be exposed in the command palette:

vscode-powershell/package.json

Lines 331 to 334 in d6446de

	{
	"command": "PowerShell.InvokeRegisteredEditorCommand",
	"when": "false"
	}

But because of the false there it never was. Changing it to true does expose it, but it fails:

Command 'PowerShell: Invoke Registered Editor Command' resulted in an error (Running the contributed command: 'PowerShell.InvokeRegisteredEditorCommand' failed.)

Additionally, I attempted to follow the documented examples. While registration of commands appears to work, and some of simple examples work (such as MyModule.MyCommandWithFunction), complicated commands like MyModule.MyEditCommand which expect a working context do not currently work.

Finally, as previously noted in PowerShell/PowerShellEditorServices#1444, all of the tests for this functionality were long ago disabled.

So it's hard for me to say that this functionality is supported. Clearly it's not in use enough to be receiving bug reports despite being broken.

Proposed Design

Personally, in the name of simplification, I default to removing code that is not in use, not covered in tests, and not contributing to the core extension experience. However, I'm opening this Request For Comment because it is possible others are using this (somehow). If this code is to remain around, at the bare minimum the tests need to be brought back online. I'm not saying I'm about to delete this code right now, but I am saying I don't currently see why or how it should continue to be maintained considering its current state, hence I'm seeking feedback/education/help.

Pinging @TylerLeonhardt, @daviwil, @rjmholt, @SeeminglyScience, @TobiasPSP for comments.

Thanks in advance!

[SeeminglyScience]

I found this as I was going through our package's contributed commands, specifically one of the entry points to it: InvokeRegisteredEditorCommand. In #2225 this command was supposed to be exposed in the command palette:

I'll include my response from that thread for completeness:

@TylerLeonhardt As far as I can tell this means that the command will never be shown in the command palette...so this PR didn't do what it intended. 🧐

I believe it's just the description that is off. This PR enabled it's usage in keybinds, e.g. this failed before it:

{
    "key": "alt+shift+s",
    "command": "PowerShell.InvokeRegisteredEditorCommand",
    "args": { "commandName": "ConvertTo-SplatExpression" },
    "when": "editorLangId == 'powershell'"
},

Since it has parameters, and "Show Additional Commands from PowerShell Modules" fills the command palette role, I believe this was intended.

---

Additionally, I attempted to follow the documented examples. While registration of commands appears to work, and some of simple examples work (such as MyModule.MyCommandWithFunction), complicated commands like MyModule.MyEditCommand which expect a working context do not currently work.

Yeah you gotta assembly qualify the type due to ALC weirdness. Or just type it as object which is what I've seen most often. Docs are a bit jacked.

it is possible others are using this

Yeah I use it extensively in EditorServicesCommandSuite. I've seen it in a few profiles as well, a lot of the more simple editor commands don't get published. Makes it real hard to tell exactly how wide spread the usage is (though plenty of folks use ESCS anyway). There are also folks who just use the $psEditor API without editor commands which still works as it did before pretty much.

@adamdriscoll might use it in his addon as well.

[TylerLeonhardt]

The $psEditor object is also "backcompat" for the $psISE variable in the ISE.

[andyleejordan]

Got the tests re-enabled at least.