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

Fixes to default selected SCM code #6426

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

westbury
Copy link
Contributor

@westbury westbury commented Oct 21, 2019

What it does

There are a couple of related issues here that are fixed.

  1. When setting the selected repository in ScmService to undefined, it does not set it to undefined but sets it to the first repository in its list. This prevents extenders from setting the selection to be 'no selection'. It is generally considered bad practice to put business logic in setters so I moved this out of the setter.

  2. When one has two or more SCM provider implementations then the setting of the selection fails. This is because if the selected repository is not a Git repository then the Git provider sets the repository to undefined (because toScmRepository will return undefined for repositories other than Git repositories). This forces the selected repository to the first repository in the list, overwriting the selection set by any other SCM provider.

How to test

You can test that all works the same as before when there is only the Git provider. If you want to see a problem that is fixed by this change then the steps are as follows:

  • install a second SCM provider.
  • create two projects, one in an SCM repository of each type.
  • select one and restart. Check which is shown in the SCM view.
  • select the other and restart. Check which is shown in the SCM view.

Review checklist

Reminder for reviewers

@westbury westbury added git issues related to git scm issues related to the source control manager labels Oct 21, 2019
@akosyakov
Copy link
Member

We should test that it does not break working with multiple SCM repositories contributed by VS Code extensions:

  • If a first repository gets added it should be mark as selected.
  • If a selected repository gets removed then another one has to be selected.

@westbury
Copy link
Contributor Author

  • If a selected repository gets removed then another one has to be selected.

This case is currently broken in master. To see this:

  1. install mrcrowl.hg vscode extension
  2. create at least one Hg project and one Git project
  3. select the Hg project
  4. delete the Hg project
    You will see the Hg project is still selected even though it does not exist.

This PR will not fix this particular case. It does fix the problem in the case where the Hg project is created using a native Theia extension that handles selections properly. Unfortunately the experience will never be as good with a VSCode extension as it is with a Theia extension.

@akosyakov
Copy link
Member

@westbury I meant only with the git built-in extension from VS Code without Theia git extension, and with several git repositories.

VS Code git extension can be downloaded here: https://registry.npmjs.org/@theia/vscode-builtin-git/-/vscode-builtin-git-0.2.1.tgz It should be enough to put it under plugins folder and remove @theia/git from the app.

@akosyakov
Copy link
Member

This case is currently broken in master.

It would be good to file a follow-up issue.

Copy link
Contributor

@vinokurig vinokurig left a comment

Choose a reason for hiding this comment

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

I've tested this with the built-in VsCode Git plugin, the folowing use-cases work as expected:

  • If a first repository gets added it should be mark as selected.
  • If a selected repository gets removed then another one has to be selected.
  • If a single repository gets removed then no repository has to be selected.

@westbury westbury merged commit a873de0 into eclipse-theia:master Oct 30, 2019
@westbury westbury deleted the selected-scm branch October 30, 2019 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
git issues related to git scm issues related to the source control manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants