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

[vscode] test and fix bogus param combinations to open dialogs #4797

Open
akosyakov opened this issue Apr 3, 2019 · 5 comments
Open

[vscode] test and fix bogus param combinations to open dialogs #4797

akosyakov opened this issue Apr 3, 2019 · 5 comments
Labels
bug bugs found in the application dialogs issues related to dialogs vscode issues related to VSCode compatibility

Comments

@akosyakov
Copy link
Member

akosyakov commented Apr 3, 2019

Right now there seem to be missing cases for open dialogs when they are called from VS Code extensions. It would be good to set up a small VS Code extension contributing several commands with different options combinations, run it against Theia and VS Code side by side and fix missing cases and differences.

Motivated by #4763

@akosyakov akosyakov added bug bugs found in the application dialogs issues related to dialogs vscode issues related to VSCode compatibility labels Apr 3, 2019
@benoitf
Copy link
Contributor

benoitf commented Apr 3, 2019

hello, could you list your current missing case ?

@benoitf
Copy link
Contributor

benoitf commented Apr 3, 2019

sorry, hit enter too soon. I wanted to bring source data in the notification. Is it part of your issue ?

@akosyakov
Copy link
Member Author

akosyakov commented Apr 3, 2019

My problem that there is no way to verify right now that there are no such cases. Looking at the code we don't have internal clients for some combinations and i doubt they work properly called from VS Code extensions. i.e. with disable canSelectFolders but enabled multiple selection it can leak actually folders.

@vzhukovs
Copy link
Contributor

vzhukovs commented Apr 5, 2019

@akosyakov I think, that here check condition should be improved to verify whether property canSelectFolders is not undefined. Because user can omit this property and value !undefined will be true.

@akosyakov
Copy link
Member Author

@vzhukovskii ok, but we really need tests, not fixing it blindly, another possible dialog issue: #4820 (comment)

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 dialogs issues related to dialogs vscode issues related to VSCode compatibility
Projects
None yet
Development

No branches or pull requests

3 participants