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

DocumentsMainImpl.toEditorOpenerOptions should convert ViewColumn to zero-based index #6801

Closed
godlin-gh opened this issue Dec 30, 2019 · 2 comments
Labels
bug bugs found in the application help wanted issues meant to be picked up, require help plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility

Comments

@godlin-gh
Copy link
Contributor

godlin-gh commented Dec 30, 2019

Description

when i call plugin api window.showTextDocument, the call hierarchy looks like:
DocumentsMainImpl.$tryShowDocument -> DocumentsMainImpl.toEditorOpenerOptions

async $tryShowDocument(uri: UriComponents, options?: TextDocumentShowOptions): Promise<void> {
// Removing try-catch block here makes it not possible to handle errors.
// Following message is appeared in browser console
// - Uncaught (in promise) Error: Cannot read property 'message' of undefined.
try {
const editorOptions = DocumentsMainImpl.toEditorOpenerOptions(this.shell, options);
const uriArg = new URI(CodeURI.revive(uri));
const opener = await this.openerService.getOpener(uriArg, editorOptions);
await opener.open(uriArg, editorOptions);
} catch (err) {
throw new Error(err);
}
}

} else if (viewColumn > 0) {
const tabBars = shell.mainAreaTabBars;
const tabBar = tabBars[viewColumn];
if (tabBar && tabBar.currentTitle) {
widgetOptions = { ref: tabBar.currentTitle.owner };
}
}

the ViewColumn type use one-based index which should be convert to zero-based index inside toEditorOpenerOptions method

Reproduction Steps

try to call window.showTextDocument(xx, { viewColumn: ViewColumn.One }), you can never open an editor on first column.

OS and Theia version:
master
Diagnostics:

@godlin-gh
Copy link
Contributor Author

godlin-gh commented Dec 30, 2019

I found there are two other reference of toEditorOpenerOptions method at plugin-vscode-commands-contribution.ts

one of that convert the viewColumn to zero-based index using fromViewColumn method outside toEditorOpenerOptions like below:

registerCommands(commands: CommandRegistry): void {
commands.registerCommand(VscodeCommands.OPEN, {
isVisible: () => false,
execute: async (resource: URI, columnOrOptions?: ViewColumn | TextDocumentShowOptions) => {
if (!resource) {
throw new Error(`${VscodeCommands.OPEN.id} command requires at least URI argument.`);
}
if (!URI.isUri(resource)) {
throw new Error(`Invalid argument for ${VscodeCommands.OPEN.id} command with URI argument. Found ${resource}`);
}
let options: TextDocumentShowOptions | undefined;
if (typeof columnOrOptions === 'number') {
options = {
viewColumn: fromViewColumn(columnOrOptions)
};
} else if (columnOrOptions) {
options = {
...columnOrOptions,
viewColumn: fromViewColumn(columnOrOptions.viewColumn)
};
}
const editorOptions = DocumentsMainImpl.toEditorOpenerOptions(this.shell, options);
await open(this.openerService, new TheiaURI(resource), editorOptions);
}
});

I think assigning 0 value to a viewColumn may be confused, we shoud do that inside the toEditorOpenerOptions instead and make sure it works on other place.

@akosyakov akosyakov added bug bugs found in the application help wanted issues meant to be picked up, require help plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility labels Dec 30, 2019
@akosyakov
Copy link
Member

@godlin-gh it sounds reasonable, feel free to work on it.

godlin-gh pushed a commit to godlin-gh/theia that referenced this issue Jan 6, 2020
… toEditorOpenerOptions() method

Signed-off-by: Gan Lin <[email protected]>
akosyakov pushed a commit to akosyakov/theia that referenced this issue Feb 24, 2020
… toEditorOpenerOptions() method

Signed-off-by: Gan Lin <[email protected]>
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 help wanted issues meant to be picked up, require help plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility
Projects
None yet
Development

No branches or pull requests

2 participants