Overwrite editor options for vscode.open and vscode.diff #110398
Conversation
|
PR up for discussion. I think less controversial is https://github.com/microsoft/vscode/pull/110398/files#diff-9bc927a30b92207292bb17a10e58c2ffe05cc22d5ea5ddcd7595cb19dbe50696 where all the logic of rewriting editor options is handled within the API command so that we can reuse this in other places like SCM and timeline. @jrieken need some advice if this direction is OK for the second part. My head spins a bit because I am changing code that I do not have full knowledge of, namely:
|
| // this enables `vscode.open` and `vscode.diff` to benefit | ||
| // from the actual editor options to use based on the context | ||
| // (e.g. invocation from a custom tree view) | ||
| result.arguments = command.arguments; |
There was a problem hiding this comment.
I have tried the same thing right now and I can report that it doesn't work... This is what now happens
- the arguments go through normal JSON-serialization (which for vscode.Range is special)
- the command get send to the renderer
- the renderer has
vscode.openregistered as well (need @alexdima to refresh my memory here) - when selecting an item in the tree the renderer-side
vscode.opencommand is invoked and it fails to convert the selection/range argument
There was a problem hiding this comment.
It seems have not coordinated well - sorry for the duplication. I have pushed some work here: https://github.com/microsoft/vscode/compare/joh/open
The idea is that CommandConverter#toInternal doesn't only create/feed the delegate command but that it actually converts some (API) commands to the internal variant. E.g a tree item will then be using _workbench.open and so on
There was a problem hiding this comment.
@jrieken What can I help with?
There was a problem hiding this comment.
There is src/vs/workbench/api/common/apiCommands.ts which registers API commands on both sides - renderer and extension host. Not sure why that was done. Also, it registers commands that accept arguments which are types of vsocde.d.ts which means they "don't survive" the JSON serialisation/deserialisation, like vscode.Selection.
There was a problem hiding this comment.
I remember... We made API command registration extension host only so that we can have multiple extension host (the global flag) but then we hit bugs with API-commands that escaped the extension host without treatment, like in markdown and then we added some API-commands to the renderer as well. It seems that some have evolved into a state where they only partially work in the renderer
There was a problem hiding this comment.
@jrieken I think your changes in https://github.com/microsoft/vscode/compare/joh/open are cleaner, so I am fine to drop my work and only keep the special massaging of the 2 commands on the main side. To clarify, this would mean we end up having _workbench.open as command in the tree and not vscode.open? I actually prefer that because now I need to depend on some internal API knowledge in the tree. I would rather depend on _workbench.open 👍
There was a problem hiding this comment.
Yeah, it will be _workbench.open and it's friends. I have created #110455 which I am planning to merge despite some polish of where to revive uris and so
|
Ok, actual solution is now in #110475 |
This PR fixes #85636