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

showTextDocument doesn't work with URI argument from editor/context menu command #13949

Open
t1m0thyj opened this issue Jul 24, 2024 · 3 comments · May be fixed by #13961
Open

showTextDocument doesn't work with URI argument from editor/context menu command #13949

t1m0thyj opened this issue Jul 24, 2024 · 3 comments · May be fixed by #13961
Labels
bug bugs found in the application vscode issues related to VSCode compatibility

Comments

@t1m0thyj
Copy link

t1m0thyj commented Jul 24, 2024

Bug Description:

The vscode.window.showTextDocument method supports only Theia URIs, and is incompatible with the VSCode URI passed as an argument when an editor/context menu command is executed.

Steps to Reproduce:

  1. Create a VS Code extension that contributes an editor/context menu command named theia.sampleCommand, and registers a handler like the following:
    vscode.commands.registerCommand("theia.sampleCommand", async (uri) => {
        // Ensure document is open and not in preview mode
        vscode.window.showTextDocument(uri, { preview: false });
        // Do other stuff
        vscode.window.showInformationMessage(uri.toString());
    });
  2. Install the extension in Theia and trigger the command by clicking it in editor context menu.
  3. Note that the info message never appears, and there is an error in the console:
    Uncaught (in promise) 
    TypeError: Cannot read properties of undefined (reading 'toString')
        at DocumentsExtImpl.showDocument (:3000/home/theia/nod…documents.js:192:63)
        at Object.showTextDocument (:3000/home/theia/nod…n-context.js:232:33)
        at Object.<anonymous> (:3000/home/theia/.th…ension.js:33:299442)
        at Generator.next (<anonymous>)
        at :3000/home/theia/.th…ension.js:33:276506
        at new Promise (<anonymous>)
        at r (:3000/home/theia/.th…ension.js:33:276251)
        at t.submitJcl (:3000/home/theia/.th…ension.js:33:299223)
        at Object.<anonymous> (:3000/home/theia/.th…ension.js:90:501340)
        at Generator.next (<anonymous>)
    

Additional Information

  • Operating System: Alpine Linux (Docker)
  • Theia Version: 1.49.1

I believe I've tracked this down to the following line in showTextDocument, which has an instanceof check for Theia URIs:

const uri: URI = documentArg instanceof URI ? documentArg : documentArg.uri;

When editor/context menu command is executed, the argument passed is an instance of CodeUri which is populated by the getSelectedResources method:

protected getSelectedResources(): [CodeUri | TreeViewItemReference | undefined, CodeUri[] | undefined] {
const selection = this.selectionService.selection;
const resourceKey = this.resourceContextKey.get();
const resourceUri = resourceKey ? CodeUri.parse(resourceKey) : undefined;
const firstMember = TreeWidgetSelection.is(selection) && selection.source instanceof TreeViewWidget && selection[0]
? selection.source.toTreeViewItemReference(selection[0])
: UriSelection.getUri(selection)?.['codeUri'] ?? resourceUri;
const secondMember = TreeWidgetSelection.is(selection)
? UriSelection.getUris(selection).map(uri => uri['codeUri'])
: undefined;
return [firstMember, secondMember];
}

Since the URI passed to the command handler is a VSCode URI (from @theia/core/shared/vscode-uri), it fails the instanceof check that expects a Theia URI, resulting in document.uri evaluating to undefined.

@msujew
Copy link
Member

msujew commented Jul 24, 2024

Hey @t1m0thyj, thanks for the report and the investigation on the issue.

Since you already looked into this, are you interested in contributing a fix?

@msujew msujew added bug bugs found in the application vscode issues related to VSCode compatibility labels Jul 24, 2024
@t1m0thyj
Copy link
Author

@msujew Sure, I'd be happy to. Which approach would you prefer for a fix?

  1. Make the instanceof check less restrictive by checking for VS Code URI
  2. Update the getSelectedResources method to return an array of Theia URIs

I'm leaning towards the first option since it seems like a less impactful change.

@msujew
Copy link
Member

msujew commented Jul 24, 2024

@t1m0thyj I also think the first option is better 👍

@t1m0thyj t1m0thyj linked a pull request Jul 25, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants