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

Fix creating a new file or folder if the preferences widget is opened. #7302

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

Anasshahidd21
Copy link
Contributor

What it does

Fixes: #7067 , #7253

Initially we were unable to create a new file or folder while the user preferences was open.

The user-preferences has the file scheme set as user-storage and we did not fall back to the workspace root URI and instead were getting the URI from user_storage:settings.json.
With this approach, we create files at certain directories and if a directory doesn't have the scheme set to file it will fallback to the workspace root uri.

Signed-off-by: Muhammad Anas Shahid muhammad.shahid@ericsson.com

How to test

  • Launch Theia

  • Open any workspace

  • Go to File>Settings> Open Preferences

  • Make sure you are on user tab

  • Try creating a new file or folder from File>New File or File> New Folder.

Review checklist

Reminder for reviewers

@Anasshahidd21 Anasshahidd21 changed the title Cannot create a new file if the preferences widget is opened. Fix creating a new file or folder if the preferences widget is opened. Mar 9, 2020
@vince-fugnitto vince-fugnitto added bug bugs found in the application preferences issues related to preferences labels Mar 9, 2020
@vince-fugnitto vince-fugnitto requested a review from lmcbout March 9, 2020 18:04
@lmcbout
Copy link
Contributor

lmcbout commented Mar 10, 2020

If no File or Folder is selected, then the new File | Folder should be created under the first root folder of the workspace, otherwise, the new File | Folder should be created under the selected folder. With a multi-root workspace, the new File or Folder can be created under the other root workspace if the focus is on the second root folder

@vince-fugnitto
Copy link
Member

If no File or Folder is selected, then the new File | Folder should be created under the first root folder of the workspace, otherwise, the new File | Folder should be created under the selected folder. With a multi-root workspace, the new File or Folder can be created under the other root workspace if the focus is on the second root folder

@lmcbout I'm not sure I understand your comment, when a uri cannot be inferred (for example in the case of opening the preferences widget), then we fallback to the first workspace root. I think this is a fair fallback.

@lmcbout
Copy link
Contributor

lmcbout commented Mar 10, 2020

If the preference setting is open and the tab is on "User" pref, the "New File | New Folder" crete it in th workspace, but when I select the Preference setting with the "workspace selected", it creates "New File | New Folder" under my /.theia folder which is outside the workspace. It just like you change the order of where the file is created

@vince-fugnitto
Copy link
Member

If the preference setting is open and the tab is on "User" pref, the "New File | New Folder" crete it in th workspace, but when I select the Preference setting with the "workspace selected", it creates "New File | New Folder" under my /.theia folder which is outside the workspace. It just like you change the order of where the file is created

It will create a new file/folder wherever the workspace settings is located.
In the case of an untitled workspace file, it will be located in the home.
We can improve the behavior for such a case easily however.

@lmcbout
Copy link
Contributor

lmcbout commented Mar 10, 2020

If the preference setting is open and the tab is on "User" pref, the "New File | New Folder" crete it in th workspace, but when I select the Preference setting with the "workspace selected", it creates "New File | New Folder" under my /.theia folder which is outside the workspace. It just like you change the order of where the file is created

It will create a new file/folder wherever the workspace settings is located.
In the case of an untitled workspace file, it will be located in the home.
We can improve the behavior for such a case easily however.

Sorry, it create it under <$Home>/.theia , not under the workspce/.theia

@vince-fugnitto
Copy link
Member

Sorry, it create it under <$Home>/.theia , not under the workspce/.theia

I understood, untitled workspace (Untitled.theia-workspace) are created under the home and it is the reason why it is displayed.

@lmcbout
Copy link
Contributor

lmcbout commented Mar 10, 2020

@lmcbout I'm not sure I understand your comment, when a uri cannot be inferred (for example in the case of opening the preferences widget), then we fallback to the first workspace root. I think this is a fair fallback.

I agree with you. When you open the preference settings , no other files, then creating a new "File | Folder" should be done in the first root folder

@Anasshahidd21
Copy link
Contributor Author

@lmcbout I have updated the code, if you can review.

Fixes: eclipse-theia#7067 , eclipse-theia#7253

Initially we were unable to create a new file or folder while the user preferences was open.

The user-preferences has the file scheme set as `user-storage` and we did not fall back to the workspace root URI and instead were getting the URI from 'user_storage:settings.json'.
With this approach, we create files at certain directories and if a directory doesn't have the scheme set to 'file' it will fallback to the workspace root uri.

Signed-off-by: Muhammad Anas Shahid <muhammad.shahid@ericsson.com>
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

LGTM

@akosyakov
Copy link
Member

anything else has to be done before merging?

@Anasshahidd21
Copy link
Contributor Author

anything else has to be done before merging?

@akosyakov no, its complete imo.

@vince-fugnitto
Copy link
Member

anything else has to be done before merging?

@akosyakov no, its complete imo.

@lmcbout I think you can merge

@lmcbout lmcbout merged commit 01394fb into eclipse-theia:master Mar 25, 2020
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 preferences issues related to preferences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create a new file if the preferences widget is opened.
4 participants