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.

Sign up for GitHub

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

Jump to bottom

scm: add 'scm.defaultViewMode' preference #7717

Merged
merged 1 commit into from
May 4, 2020
Merged

scm: add 'scm.defaultViewMode' preference #7717

merged 1 commit into from
May 4, 2020

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes: #7713

The following pull-request adds a new preference for the scm extension named
scm.defaultViewMode which is used to control the view mode rendering of the scm widget between displaying results as a tree or a flat list.

scm.defaultViewMode: list

Screen Shot 2020-05-01 at 9 47 58 AM

scm.defaultViewMode: tree

Screen Shot 2020-05-01 at 9 47 50 AM

How to test

  1. open a workspace with changes under version-control
  2. open the scm widget
  3. update the preference scm.defaultViewMode
    (the scm widget's view mode should update accordingly)

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added preferences issues related to preferences scm issues related to the source control manager labels May 1, 2020
@vince-fugnitto vince-fugnitto self-assigned this May 1, 2020
@vince-fugnitto
Copy link
Member Author

@westbury internally I kept flat over list (list is used in the VS Code preferences for the viewMode). I did not want to break ARM if perhaps you are referring to flat as a value.

@vince-fugnitto
scm: add 'scm.defaultViewMode' preference
c9b88e2 
added the preference `scm.defaultViewMode` which controls the
view mode rendering of the scm widget from tree to flat list.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
westbury
westbury approved these changes May 1, 2020
View reviewed changes
Copy link
Contributor

@westbury westbury left a comment

Choose a reason for hiding this comment

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

I have tested it and it works great. Thanks. I'm impressed that changing the preference even changes views that are already open.

BTW, sorry earlier I commented on code without first reading the full PR. I didn't see your PR comment until later. I'll be more careful to read the PR before commenting on code linked to in the e-mail. Thanks for worrying about breaking Arm but actually it is a lot of work for us to upgrade Theia and a change like that would be trivial for us.

@vince-fugnitto
Copy link
Member Author

I have tested it and it works great. Thanks. I'm impressed that changing the preference even changes views that are already open.

BTW, sorry earlier I commented on code without first reading the full PR. I didn't see your PR comment until later. I'll be more careful to read the PR before commenting on code linked to in the e-mail. Thanks for worrying about breaking Arm but actually it is a lot of work for us to upgrade Theia and a change like that would be trivial for us.

Thank you for the review and no problem! :)

@westbury
Copy link
Contributor

westbury commented May 4, 2020

@vince-fugnitto is it ok if I merge this? The reason I ask is that I have another PR that would conflict with this one.

@vince-fugnitto
Copy link
Member Author

@vince-fugnitto is it ok if I merge this? The reason I ask is that I have another PR that would conflict with this one.

Yes go ahead!

@vince-fugnitto vince-fugnitto merged commit 0a691e2 into master May 4, 2020
@vince-fugnitto vince-fugnitto deleted the vf/scm-preferences branch May 4, 2020 14:51
@vince-fugnitto
Copy link
Member Author

@westbury I merged for you, thank you for the review :)

lmcbout
lmcbout approved these changes May 4, 2020
View reviewed changes
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.

Tested on Vhrome and Electron
Looks nice
I prefer the tree look, but I guess I just need to set it by default in my preferences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westbury westbury westbury approved these changes

@lmcbout lmcbout lmcbout approved these changes

Assignees

@vince-fugnitto vince-fugnitto

Labels
preferences issues related to preferences scm issues related to the source control manager
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Source Control view should have preference for default mode ('tree' or 'flat')
3 participants
@vince-fugnitto @westbury @lmcbout