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

Add ability to close editor and retrieve document without opening the last one #5080

Merged
merged 1 commit into from
May 7, 2019

Conversation

vzhukovs
Copy link
Contributor

@vzhukovs vzhukovs commented May 6, 2019

This changes proposal adds small ability not to open editor if text document show options is undefined. This need to retrieve document object without opening the last one in persistence editor. This how vs code does in default behavior. This method, according to default vs code behavior should not open editor, but return document object. Sample use case, is just getting document content and open it in web view. Without this fix we can get two opened editors. To avoid huge refactoring in plugin's documents and editors related code, there was decided to add simple checks if text document show options is undefined, then we don't keep opened editor.

Signed-off-by: Vladyslav Zhukovskyi vzhukovs@redhat.com

@vzhukovs vzhukovs added bug bugs found in the application plug-in system issues related to the plug-in system Team: Che-Plugins issues related to the che-plugins team labels May 6, 2019
@vzhukovs vzhukovs requested a review from evidolob May 6, 2019 12:44
@vzhukovs vzhukovs requested a review from benoitf as a code owner May 6, 2019 12:44
@vzhukovs vzhukovs self-assigned this May 6, 2019
@vzhukovs
Copy link
Contributor Author

vzhukovs commented May 6, 2019

During the resolving this problem another issue was created: #5079

@@ -248,6 +248,13 @@ export class DocumentsExtImpl implements DocumentsExt {
};
}
await this.proxy.$tryOpenDocument(uri, documentOptions);

if (!options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it worth mentioning in the method's doc that omitting the options parameter actually means opening a document without showing it on UI? To make it a bit clearer...
And adding the link here to the related issue might help someone quicker understand why it has been implemented in such way and this is actually a temporary fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment with dependent issue and jsdoc

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to merge it as a temporary fix

@vzhukovs
Copy link
Contributor Author

vzhukovs commented May 6, 2019

Related issue: eclipse-che/che#13037

… last one

Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
@vzhukovs vzhukovs merged commit 4e37479 into master May 7, 2019
@vzhukovs vzhukovs deleted the che#13037 branch May 7, 2019 13:26
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 plug-in system issues related to the plug-in system Team: Che-Plugins issues related to the che-plugins team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants