-
Notifications
You must be signed in to change notification settings - Fork 665
Hide 'irrelevant' upstream commits #9676
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
base: master
Are you sure you want to change the base?
Conversation
Updated the branch details extraction logic in crates/but-workspace/src/stacks.rs: - Changed map call to pass repository context to from_segment method. - Enhanced from_segment method in ui::BranchDetails to accept repository reference. - Added computation of commits and last_local_commit. - Filtered upstream commits by checking for tree changes against last local commit. - Returned upstream commits with changes only. - Adjusted construction of BranchDetails to use new commits and upstream_commits variables. These changes improve the granularity of branch details by including information about commits and relevant upstream changes.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Do we want this? This is better for the following use-case: I pushed a branch so all my commits are both local and remote. What we do nowWe display an upstream only commit representing the commit that 'disappeared' after the squash What would make it less confusingWe don't display that 'upstream only' commit. Because in even if that's correctly picked up as an upstream-only commit it is very much not an upstream-only change. The change is also available locally, and hence there is nothing to integrate. DrawbacksIf we want to be an honest-to-god WYSIWYG git client we'd show this for what it is: A branch divergence. My guess is that we just need to find a sweet-spot in between pure git client and GitButler magic |
First of all, being smarter in hiding irrelevant things to me seems very valuable, and we should have more of that. Instead of using magic tricks, I think we should use science to actually bend the spoon - only that way the user can't come and break the illusion (or run into things that clearly aren't correct). When trying this branch, I was very confused myself, somehow the order of the squash matters. Also, it's very specific to this one case, which might make people think it works just to not work a moment later. Screen.Recording.2025-08-04.at.13.26.34.movHowever, I think this can work, with more work, and then it would have to be handled similar to the existing squash-checks in conjunction with the integration branch. It would require plenty of tests as well which fortunately can now be created quite easily, just so that we consciously set the bar for "it works for 95%". These tests would have to be added in the function that creates the Another approach to this, instead of using changeset ids, could be to use change-ids and allow each commit to have more than one. Then the squashed commit would remember the commits that it represents. With that I am just afraid that the user adds more changes to it so these commits go out of sync even more, but maybe that's still what's logical, and something worth trying out. |
In case it's unclear: I have no objections to merge this, it's just something that may eventually fall by the wayside without a test. |
Updated the branch details extraction logic in crates/but-workspace/src/stacks.rs:
These changes improve the granularity of branch details by including information about commits and relevant upstream changes.