Skip to content

Only discard the files changes inside the lane #4820

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
Merged
merged 5 commits into from
Sep 5, 2024
Merged

Only discard the files changes inside the lane #4820

merged 5 commits into from
Sep 5, 2024

Conversation

estib-vega
Copy link
Contributor

@estib-vega estib-vega commented Sep 3, 2024 •
edited
Loading

Problem

When using the file context menu inside a given lane to discard the changes, all changes to that file across all lanes are discarded.
This is either weird or unexpected behavior.

Solution

Only discard the changes inside the lane that the context menu was opened from.

Issues

This fixes issue #4137

@estib-vega
Only discard the files changes inside the lane
34e102f 
When using the file context menu inside a given lane to discard the changes, only the changes to that file inside the lane should be discarded.
Any other changes to that same file that live in other lanes are kept
@vercel Vercel
Copy link

vercel bot commented Sep 3, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gitbutler-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 0:28am

@github-actions github-actions bot added rust Pull requests that update Rust code @gitbutler/desktop labels Sep 3, 2024
@estib-vega estib-vega force-pushed the unapply-only-hunks-from-lane branch from 8f6330d to 394f1d9 Compare September 3, 2024 09:47
mtsgrd
mtsgrd reviewed Sep 3, 2024
View reviewed changes
crates/gitbutler-branch-actions/src/virtual.rs Outdated
claims: filtered_ownership_claims,
}
})
.context("failed to determin ownership to unapply")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops!
Fixed :)

@estib-vega estib-vega force-pushed the unapply-only-hunks-from-lane branch from 394f1d9 to b3c96ab Compare September 3, 2024 14:43
@Byron
refactor backend of reset_files() to be more idiomatic
50c75ee 
It also tries to use `Vec<PathBuf>` in the tauri interface, which is
still to be validated.
Byron
Byron reviewed Sep 4, 2024
View reviewed changes
Copy link
Collaborator

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks so much for all your recent work here (making missing projects deletable was great!).

I took the liberty to edit the rust-side of things so that it's idiomatic and less wasteful. However, I also tried something new which is to pass Vec<PathBuf> from the frontend to the backend directly. In the frontend, this means that a string-array should be passed and I think it should convert to Vec<PathBuf> automatically. However, I didn't try it as I didn't adjust the frontend code.

It would be great if it could work like this, but if not, Vec<String> will be fine, and a conversion to Vec<PathBuf> in the backend.

Thanks again!

@estib-vega
Copy link
Contributor Author

Hey @Byron! Your improvements are more than welcome haha.
I try to adjust the FE code accordingly and give it a go.

Thanks!

@mtsgrd mtsgrd added the e2e-testing End-to-end testing related things label Sep 4, 2024
@mtsgrd
Copy link
Contributor

mtsgrd commented Sep 4, 2024

I added an e2e testing label here as a reminder this might be a good thing to cover.

@estib-vega estib-vega force-pushed the unapply-only-hunks-from-lane branch from 186a8c4 to c93db0e Compare September 4, 2024 09:11
@estib-vega estib-vega force-pushed the unapply-only-hunks-from-lane branch from c93db0e to 50c75ee Compare September 4, 2024 09:37
ndom91
ndom91 approved these changes Sep 5, 2024
View reviewed changes
Copy link
Contributor

@ndom91 ndom91 left a comment

Choose a reason for hiding this comment

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

LGTM 🥳

@estib-vega estib-vega merged commit 000bddb into master Sep 5, 2024
16 checks passed
@estib-vega estib-vega deleted the unapply-only-hunks-from-lane branch September 5, 2024 09:02
@krlvi krlvi mentioned this pull request Sep 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndom91 ndom91 ndom91 approved these changes

@Byron Byron Awaiting requested review from Byron

@Caleb-T-Owens Caleb-T-Owens Awaiting requested review from Caleb-T-Owens

@mtsgrd mtsgrd Awaiting requested review from mtsgrd

@krlvi krlvi Awaiting requested review from krlvi

Assignees
No one assigned
Labels
e2e-testing End-to-end testing related things @gitbutler/desktop rust Pull requests that update Rust code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@estib-vega @mtsgrd @Byron @ndom91