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

Add multi-select push and pull to tracked explorer tree #1809

Merged
merged 8 commits into from
Jun 2, 2022

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Jun 1, 2022

1/2 main <- this <- #1810

Closes #1668.

This PR adds multi-select push/pull actions to the tracked explorer tree.

Demo

Screen.Recording.2022-06-01.at.3.59.22.pm.mov

Note: Once we bump the min version of DVC the number of files being pulled should be reduced as the exp show but will be squashed.

@mattseddon mattseddon added the product PR that affects product label Jun 1, 2022
@mattseddon mattseddon self-assigned this Jun 1, 2022
): TreeView<string | T> =>
window.createTreeView<string | T>(name, {
canSelectMany: false,
canSelectMany,
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is a lie from the VS Code API docs:

image

Copy link
Member Author

@mattseddon mattseddon Jun 1, 2022

Choose a reason for hiding this comment

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

Because this doesn't work I have used the tree's selection property.

}

private getSelectedPathItems() {
return [...this.treeView.selection]
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] We can use a similar approach to delete multiple experiments from the experiments tree.

const acc = []
const acc: string[] = []
if (typeof pathItem === 'string') {
return acc
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] If we try to pull the root then just run dvc pull with the root as the cwd.

@mattseddon mattseddon marked this pull request as ready for review June 1, 2022 06:06
@rogermparent
Copy link
Contributor

Seems that this breaks single-item pulls as well as the pull button in the SCM tree.

single-pull-broken-demo.mp4

@mattseddon
Copy link
Member Author

Seems that this breaks single-item pulls as well as the pull button in the SCM tree.

I will fix.

@mattseddon mattseddon force-pushed the add-multi-select-push-pull branch from ad29a7f to 57d1fa8 Compare June 2, 2022 04:09
@mattseddon
Copy link
Member Author

Seems that this breaks single-item pulls as well as the pull button in the SCM tree.

@rogermparent I completely forgot that the SCM view is tied to this command so thanks. There is some history in how the commands have evolved over the life course of the codebase. Initially the commands were built specifically for the SCM view. They've been moved around, patched, and updated so many times that they are unrecognizable from the initial intended use. With all that said the easiest thing to do (for now) is to add some extra information to the SCM tree and call the same command. I still intend to rip this out/up once we get the new data:status command. LMK if you'd like to see something different done in the interim.

Demo

Screen.Recording.2022-06-02.at.3.08.57.pm.mov
Screen.Recording.2022-06-02.at.3.09.48.pm.mov

@rogermparent
Copy link
Contributor

LMK if you'd like to see something different done in the interim.

I think as long as all the features work in a basic sense, we're good to go 👍
I tried the previously broken situations as well and they work on my end as well.

@mattseddon mattseddon force-pushed the add-multi-select-push-pull branch from 4d9f85b to 569523d Compare June 2, 2022 21:26
@mattseddon mattseddon enabled auto-merge (squash) June 2, 2022 21:27
@codeclimate
Copy link

codeclimate bot commented Jun 2, 2022

Code Climate has analyzed commit 569523d and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 96.7% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.9% (0.1% change).

View more on Code Climate.

@mattseddon mattseddon merged commit c4a0be3 into main Jun 2, 2022
@mattseddon mattseddon deleted the add-multi-select-push-pull branch June 2, 2022 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DVC-tracked tree: actions on multiple objects
3 participants