Support multi-select in Source Control view #7900

westbury · 2020-05-26T21:32:08Z

What it does

Adds support for multi-selection in the Source Control view. When multiple rows are selected, the args to the context menu will include the entire selection, but only if all the nodes in the selection are in the same group and all are files or all folders.

This is different from how vs-code works because vs-code does not have file-specific items in the context menu. Vs-code will execute the inline actions against all selected items. With this PR the inline actions only operate on the file or folder associated with the inline action, even when there is a multi-selection. This makes it consistent with the existing Theia behavior when a file is selected but an inline action is pressed for a different file. In that situation the inline action applies to the file next to the action, and the selection is left alone on the other file.

This works will with the Theia native git package. It also works with the vs-code builtin but not so well. If testing with vscode-git-1.3.0.1.vsix there is no support for vs-code's new folder actions, so those are missing. (But that is the case without this PR too). Also notice that to make multi-selection you must press up-arrow or down-arrow while holding shift. Using the mouse will work but if the second selection is a modified file (M indicator) then vscode-git-1.3.0.1.vsix results in the second range selection to be selected singularly, cancelling the range that had just been set as the selection. I have not been able to test with git-1.39.1-prel.vsix.

How to test

Test with both the Theia git extension and the vs-code git builtin. Ensure that all works well with the Theia git extension. Ensure that no existing functionality is broken when using any supported git builtin.

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

lmcbout · 2020-05-28T12:16:07Z

For now, only tested with "@theia/git" on UBUNTU 18.04
Selecting the first file in SCM open the diff file in editor.
Now selecting a list of files (UPPERCASE + left mouse click) and selecting 3~4 files.
--> Only the last file opens in the diff editor, (+ first file already opened) the other 2 files are not opened, but in SCM view, they are all in HIGHLIGHT selected (Blue colors)
IS this PR open multiple files, but only the files we specifically select? or it should open all files getting highlighted?
Note: all selected files were modified file (Having "M" as modification in the tree)

akosyakov · 2020-05-29T07:45:41Z

@lmcbout please validated against VS Code, if it behaves different then it is a bug

tsmaeder

Sorry, didn't get far with the review...perhaps the comment is helpful.

tsmaeder · 2020-05-28T08:34:49Z

packages/scm/src/browser/scm-tree-widget.tsx

-            return;
-        }
-        const groupId = parentNode.groupId;
+        const groupId = this.getGroupIdForFileNode(node);
        const group = repository.provider.groups.find(g => g.id === groupId)!;
        return group.resources.find(r => String(r.sourceUri) === node.sourceUri)!;


getGroupForFileNode(...) may return ''. Do we always find a defined group for that id?

getGroupIdForFileNode should never actually return '' as that would mean we have an invalid model. I have changed it to throw an Error instead (and also moved to the node's namespace so it can be reused in one more place).

lmcbout · 2020-05-29T13:32:56Z

@akosyakov The behavior between Theia and VSCODE is different.

Theia with Chrome browser
Select the first file in the GIT view
-> First diff editor for the first file
Select multi files in Theia/git view
-> Last file now open in diff editor

Then open context menu and select "Open" (No other options like in VSCODE)
-> Does not open the missing file not in the editor view

VSCODE
Select the first file in the GIT view
-> First diff editor for the first file
Select multi files in Git view
-> No others diff editor open
Select context menu in the Git view for the selected files
-> Some options to open the files, Select "Open changes"
-> All selected files open in a diff editor view

lmcbout · 2020-05-29T14:44:04Z

Try the multi-selection using old builtin (version 1.39.1) not version 1.44.2 which does not work.
Removing theia/git and only use the builtin
Can select and open the multi-selection when using the arrow key for the selection and select "OPen Changes: from context menu.

Using the multi selection and using the arrow key to select when the view is in a tree mode, then it does not allow to open multi-selction files,

westbury · 2020-06-01T11:03:56Z

I have pushed the following changes:

the code that opens the file on single click now checks that 'shift' or 'ctrl' is not pressed. This prevents files being opened when multi-selections are made (except for the first file which can't be helped).
I have changed some behavior to match vs-code. This makes the code a little simpler though it does introduce some weirdness which I had been trying to avoid. For example, if adding a folder to .gitignore it does not add the folder but adds individually the files that happen to be in the folder at the time the action is taken. Another weirdness is that one can execute all file actions against a folder by selecting both the folder and a file then right-clicking on the file. Extenders can fix these so this is ok.

lmcbout · 2020-06-01T12:52:09Z

Since it is supposed to work with native Theia "@Theia/git"
Only tested last commit ad11de8 with native Theia
Select multi-files using the the mouse event (CTRL + mouse Left click) OR the keyboard (UPPERCASE + UP|DOWN arrow), then open the context menu : only the following options are available:

Discard Changes
Stage Changes

--> There is no "Open..." anywhere :(

westbury · 2020-06-01T15:04:36Z

There is no "Open..." anywhere :(

Yes, you're right. Its 'isVisible' method is no longer needed and should have been removed. I've pushed a new commit with this action now back.

lmcbout · 2020-06-02T12:31:17Z

Tested with native Theia "@Theia/git"
Used last commit 7f3c96d
Test: Select some files , but not all the files under a folder, it seems that when I select the "Open File" button, it opens all files under the tree list, not just the one selected. See the video.

Second: When I single click, it opens the diff editor (OK) but when selecting a list of files, I can only select "Open file", there is no option to open each file in a diff editor. It would be appreciated.

westbury · 2020-06-02T14:44:27Z

@lmcbout thanks for testing this again.

Test: Select some files , but not all the files under a folder, it seems that when I select the "Open File" button, it opens all files under the tree list, not just the one selected. See the video.

That is because you have selected the folder. If a folder is selected then the action applies to all the files in the folder. If you also selected some of the files in the folder then those are simply removed as duplicates. This is working the same as vs-code. This can be confusing, I know, which is one of the reasons why in the original version of the PR I did not allow actions on mixed files and folders.

Second: When I single click, it opens the diff editor (OK) but when selecting a list of files, I can only select "Open file", there is no option to open each file in a diff editor. It would be appreciated.

We would have to add a new command to add 'open in a diff editor' to the context menu. I would think this should be a separate PR if this is agreed.

lmcbout · 2020-06-02T19:51:40Z

Testing With @theia/git

Selection of the whole folder is the same as VSCode: OK
When opening a file , you will note that there is an icon on the editor toolbar allowing you to select the "Open Files" | "Open Changes" : It allows us to use the diff editor and switching with the Open files: GREAT
There should be other icons on the editor tab bar, but since this extension will be replace soon by the open -vsx Git and Git-UI, it is OK the way it is right now

Testing with open-vsx GIT and Git-UI version 1.39.1

When opening a file, I noticed an error on the console:

root ERROR Child node 'file-change-tree-root' does not belong to this 'file-change-tree-root' tree.

On the editor tab, there is no icon to swith "Open file" | "Open Changes" initially until you open a second file then remove it and click on the initial file in the editor, then the icon shows up. (Note: to ease this case, I also added the "GIT History" plugin, this icon shows right away on the editor tab bar) Show in the video below. Check the icon apearing when the second file is closed.
Once you have the icon, it allows you to switch the display to open in the editor. Then the other buttons are shown and if I select the (...)(More option), it does nothing
Also, on the editor tab, the other icons are missing for

- Open changes With Previous revision
- Show Revision details
- Open Changes with Next Revision
- Toggle File Blame annotation 
- More actions

akosyakov · 2020-06-03T07:15:53Z

Would it be fine to address issues with editors tabs separately? Please file an issue for it.

westbury · 2020-06-03T09:29:28Z

Testing with open-vsx GIT and Git-UI version 1.39.1

I have confirmed that everything reported under this section happens on the master branch. It is unrelated to the changes in this PR.

lmcbout · 2020-06-03T13:32:31Z

I filled the editor tab issue separately:
For the event missing to display the icon: #7905
Icons missing on the editor tabbar: #7947

lmcbout

Tested on UBUNTU 18.04 with Chrome and electron
Mainly look at the execution, did not concentrate on code itself
Multi-selection works as long as the selected files are "Modified" files. If one of the file is a new file ("A"), then an error occurs and is tracked in issue #7961

Approved on my side, but I would like someone else try it on a different platform to confim.

akosyakov

I've used it a bit and it seems to work.

packages/scm/src/browser/scm-tree-widget.tsx

Signed-off-by: Nigel Westbury <nigelipse@miegel.org>

westbury added the scm issues related to the source control manager label May 26, 2020

akosyakov requested review from akosyakov, kittaakos, lmcbout, vince-fugnitto and tsmaeder May 28, 2020 08:09

tsmaeder reviewed May 29, 2020

View reviewed changes

westbury force-pushed the scm-multiselect branch 3 times, most recently from 3b5b85e to ad11de8 Compare June 1, 2020 10:45

westbury force-pushed the scm-multiselect branch from ad11de8 to 7f3c96d Compare June 1, 2020 15:00

westbury force-pushed the scm-multiselect branch from 7f3c96d to baa9676 Compare June 3, 2020 09:34

lmcbout approved these changes Jun 4, 2020

View reviewed changes

akosyakov approved these changes Jun 5, 2020

View reviewed changes

akosyakov reviewed Jun 5, 2020

View reviewed changes

packages/scm/src/browser/scm-tree-widget.tsx Outdated Show resolved Hide resolved

westbury force-pushed the scm-multiselect branch from baa9676 to 3f58ac4 Compare June 5, 2020 11:04

Support multi-select in Source Control view

5c09aed

Signed-off-by: Nigel Westbury <nigelipse@miegel.org>

westbury force-pushed the scm-multiselect branch from 3f58ac4 to 5c09aed Compare June 5, 2020 11:49

westbury merged commit f8491b4 into master Jun 5, 2020

westbury deleted the scm-multiselect branch June 5, 2020 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multi-select in Source Control view #7900

Support multi-select in Source Control view #7900

westbury commented May 26, 2020

lmcbout commented May 28, 2020

akosyakov commented May 29, 2020

tsmaeder left a comment

tsmaeder May 28, 2020

westbury Jun 1, 2020

lmcbout commented May 29, 2020

lmcbout commented May 29, 2020

westbury commented Jun 1, 2020

lmcbout commented Jun 1, 2020

westbury commented Jun 1, 2020 •

edited

Loading

lmcbout commented Jun 2, 2020

westbury commented Jun 2, 2020 •

edited

Loading

lmcbout commented Jun 2, 2020

akosyakov commented Jun 3, 2020

westbury commented Jun 3, 2020

lmcbout commented Jun 3, 2020

lmcbout left a comment

akosyakov left a comment

Support multi-select in Source Control view #7900

Support multi-select in Source Control view #7900

Conversation

westbury commented May 26, 2020

What it does

How to test

Review checklist

Reminder for reviewers

lmcbout commented May 28, 2020

akosyakov commented May 29, 2020

tsmaeder left a comment

Choose a reason for hiding this comment

tsmaeder May 28, 2020

Choose a reason for hiding this comment

westbury Jun 1, 2020

Choose a reason for hiding this comment

lmcbout commented May 29, 2020

lmcbout commented May 29, 2020

westbury commented Jun 1, 2020

lmcbout commented Jun 1, 2020

westbury commented Jun 1, 2020 • edited Loading

lmcbout commented Jun 2, 2020

westbury commented Jun 2, 2020 • edited Loading

lmcbout commented Jun 2, 2020

akosyakov commented Jun 3, 2020

westbury commented Jun 3, 2020

lmcbout commented Jun 3, 2020

lmcbout left a comment

Choose a reason for hiding this comment

akosyakov left a comment

Choose a reason for hiding this comment

westbury commented Jun 1, 2020 •

edited

Loading

westbury commented Jun 2, 2020 •

edited

Loading