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

UX of virtual branch commit states (discussion) #3121

Closed
krlvi opened this issue Mar 12, 2024 · 49 comments
Closed

UX of virtual branch commit states (discussion) #3121

krlvi opened this issue Mar 12, 2024 · 49 comments
Assignees
Labels
enhancement An improvement to an existing feature UX/UI Focusing on user satisfaction, usability, and overall experience

Comments

@krlvi
Copy link
Member

krlvi commented Mar 12, 2024

Background:

The nature of branches having a local and remote equivalent means that commits can be in multiple states.

  1. If the local and remote are identical, there’s nothing to do
  2. If the local has commit(s) that are not on the remote, those commits can be pushed
  3. If the remote has commit(s) (from another user or the GitHub GUI) that are not local, they can be pulled
  4. If the local branch has non-punished commits and the remote has non-pulled commits the branches are diverged

I find this graphic by Julia Evans to be quite nice at illustrating this:

diverged

Problem

The lanes GitButler renders for virtual branches don’t represent the need to pull and diverged states clearlly. To illustrate this, I made a graphic showcasing the way the 4 states are shown by GitButler

Current state in GitButler

Below are 4 screenshots next to each other, covering the 4 states mentioned previously and how they are represented within GitButler virtual branches, alongside some commentary. I generated these states by making a commits using the GitHub website.

current_lanes

Comments from the graphic 👆 (the numbers in blue)

  • (1) “Upstream commits” duplicate what is local
  • (2) “Merge” is misleading as this would mean simply pulling the remote commit
  • (3) “Remote branch” is a misleading term as here we show only local commits
  • (3) “Force push to remote” doesn’t make sense as this is in the context of remote commits
  • (3) “Force push” may have unintended consequences (losing the remote commit)
  • (4) Duplicated from below
  • (5) “Merge” is a bit misleading
  • (6) “Force push to remote”, unclear difference from the other force push button, possibly unintended consequences
  • (7) Possibly unintended consequences

Lets discuss 💬

The purpose of this GH issue is to invite a discussion and maybe go over some drafts / ideas that come up. @schacon, @mtsgrd, @Qix- @PavelLaptev let me know if I have missed something

@PavelLaptev PavelLaptev added UX/UI Focusing on user satisfaction, usability, and overall experience enhancement An improvement to an existing feature labels Mar 12, 2024
@mtsgrd
Copy link
Contributor

mtsgrd commented Mar 12, 2024

@krlvi I should perhaps know the answer by now, but what is the difference between "upstream commits" and commits on "remote branch"?

@krlvi
Copy link
Member Author

krlvi commented Mar 12, 2024

@krlvi I should perhaps know the answer by now, but what is the difference between "upstream commits" and commits on "remote branch"?

I view the current state as a UI (UX) bug. In terms of what goes in each group:

  • remote branch appear to be showing commits that are both local and remote (with the teal and purple color) - this is misleading
  • upstream commits appear to be showing the branches that on the remote.

(very likely the way we have the distinction now is something for us to fully redo)

@schacon
Copy link
Member

schacon commented Mar 12, 2024

Trying to illustrate, let's say you start a new branch and commit twice (A and B), you have two "local" commits:

CleanShot 2024-03-12 at 11 47 35@2x

Then you push the branch to GitHub, now those commits are on the server as well, so we let you know that by labeling them as being on the "remote branch":

CleanShot 2024-03-12 at 11 49 22@2x

Ok, now you commit on your branch again but haven't pushed it, so now your commits are in two different groups. One that is local only and one that is pushed:

CleanShot 2024-03-12 at 11 50 11@2x

Finally, someone else pushes work to your GitHub branch (something not created locally by you). Now you have an "upstream" commit.

CleanShot 2024-03-12 at 11 50 46@2x

Now lets say you merge in the upstream commit:

CleanShot 2024-03-12 at 11 55 38@2x

Now you have a few local commits (the merged one included). Now you push, everything is "remote branch", because all the commits are on the server:

CleanShot 2024-03-12 at 13 06 33@2x

@schacon
Copy link
Member

schacon commented Mar 12, 2024

I think this is the minimally important distinction. Is a commit:

  1. present locally
  2. present on a server

If it's 1 only, it's in the "local" group

If it's 1 and 2, it's "remote branch" group

If it's only 2, it's "upstream" group

We can rename them or differently visualize them, but there are practical implications to each of these groups.

  • "local" grouped commits can be rewritten without needing to force push
  • "remote branch" grouped commits are possibly on someone else's computer and more care needs to be taken
  • "upstream" commits are not present in your working copy and could be conflicting and will need to be integrated somehow

@krlvi
Copy link
Member Author

krlvi commented Mar 12, 2024

Thanks for that illustration! I was thinking - we currently linearize the sequence, the way we show it, even when there is a merge. Perhaps that's a big source of confusion.

Since we allow merges (and many people prefer it) perhaps we can try visualising it in a non-linear way. Even as a rebaser-person I'd find it perhaps useful.

@schacon
Copy link
Member

schacon commented Mar 12, 2024

It could be interesting to pull the graph component out to the side and illustrate the topology a little bit more, rather than flattening and simplifying the relationships (esp where we have merges). jj does this in an nice way:

CleanShot 2024-03-12 at 14 01 37@2x

It would basically be git log --graph --oneline [vbranch] ish output, but prettier and a little more clear.

@KroniK907
Copy link
Contributor

KroniK907 commented Mar 12, 2024

The one state that GB allows but is not in your list of 4 states is having pushed a commit (or several) to the remote, and then undoing those commits locally. This results effectively in state 2 but requiring a force push if no changes are made after the commits are undone (eg. you just want to change the commit message). However I think once changes are made it should probably act more like state 4?

@schacon
Copy link
Member

schacon commented Mar 13, 2024

@KroniK907 in that case, the old commits become state 3 ("upstream") because they are on the server under that branch but not local. the new local work becomes state 1 ("local") as they are not upstream. You could merge them back in, the way they're done in the diagram, but more likely what you want to do is force push.

You're correct though, when that becomes problematic is when someone else pushes on top, so you want to effectively cherry pick from upstream and then force push.

@krlvi
Copy link
Member Author

krlvi commented Mar 13, 2024

I think this is the minimally important distinction. Is a commit:

present locally
present on a server

I took the example screenshots from the parent post and cropped out just the commits, leaving off the groups they were in. I also removed the duplications and just spelled out in english where the commit is.

It's crazy how much simpler the same example becomes when we skip the groups and just spell out what is where, and no duplicates:

commits

Maybe we can design something that communicates the thing that in english using shapes and colors and making it obvious at a glance what is where?

Graph idea

We could in theory draw a mini graph of the commits like this:
image
(the order of commit 3 and commit from remote 2 I presume comes from the timestamp, and of course it would be important to be able to clearly distinguish at a glance what is on my computer and what is not)

@krlvi
Copy link
Member Author

krlvi commented Mar 13, 2024

It would basically be git log --graph --oneline [vbranch] ish output, but prettier and a little more clear.

I tried the following while my vbranch is in the 4th state (diverged)
git log --graph --oneline main..$(git show-ref refs/gitbutler/testing)

* dafd306 Commit 3
* c0a106e Commit from remote
* 250db6c Commit 2
* fdd15ea Commit 1
* 828305c (origin/main) Merge pull request #12 from krlvi/Virtual-branch

If i log the origin one git log --graph --oneline main..origin/testing:

* 884e7d0 (origin/testing) Commit from remote 2
* c0a106e Commit from remote
* 250db6c Commit 2
* fdd15ea Commit 1
* 828305c (origin/main) Merge pull request #12 from krlvi/Virtual-branch

I guess it is not showing them as separate lanes because there they are not in the same place (one is in remotes).
@schacon is there a flag to have them show as 2 lanes next to each other?

@kiprasmel
Copy link

kiprasmel commented Mar 13, 2024

@krlvi

If i log the origin one git log --graph --oneline main..origin/testing:
<...>
I guess it is not showing them as separate lanes because there they are not in the same place (one is in remotes).
@schacon is there a flag to have them show as 2 lanes next to each other?

your syntax main..origin/testing is saying "everything in origin/testing, excluding everything in main (i.e. everything in origin/testing, up until merge-base with main).

what you want is: to see unique commits from both (symmetric difference). the syntax is main...origin/testing (note the 3 dots instead of 2). see "SPECIFYING RANGES" in man gitrevisions.

to visualize, could do something like:

git log --oneline --show-linear-break --left-right main...origin/testing

@krlvi
Copy link
Member Author

krlvi commented Mar 13, 2024

okay, just got some help from Scott - this one gave what i wanted git log --graph --oneline $(git show-ref refs/gitbutler/testing) origin/testing ^origin/main:

* 884e7d0 (origin/testing) Commit from remote 2
| * dafd306 (testing) Commit 3
|/
* c0a106e Commit from remote
* 250db6c Commit 2
* fdd15ea Commit 1

@krlvi
Copy link
Member Author

krlvi commented Mar 13, 2024

@PavelLaptev FYI, this graph will never have more than two tips (the diverged case, state 4)

@PavelLaptev
Copy link
Contributor

@krlvi got it. Thank you for the explanation. I'll prepare some drafts :-)

@krlvi krlvi self-assigned this Mar 18, 2024
@PavelLaptev
Copy link
Contributor

Drafts in Figma https://www.figma.com/file/FbeLt0yjY9RiNn8MXUXsYs/Client-Design?type=design&node-id=2100%3A42661&mode=design&t=lerXeTmJGwqsI4gg-1

image

We can show additional info on hover for the commit card and the avatar:

image image

@PavelLaptev PavelLaptev pinned this issue Mar 18, 2024
@krlvi
Copy link
Member Author

krlvi commented Mar 18, 2024

i really like these drafts! as we discussed irl - we could explore showing in a subtle way one more "node" in the graph for the base of this branch. This is less important than the other things so it can be more subtle, but it could be nice to be able to hover on it and see what was the base here.

Screenshot 2024-03-18 at 15 34 21

But now that I am thinking about it, this would be duplicated for all vbranches in the workspace.... so perhaps this does not belong in the lane, maybe it needs to be somehow outside?

@PavelLaptev PavelLaptev self-assigned this Mar 18, 2024
@schacon
Copy link
Member

schacon commented Mar 19, 2024

I really like this format, but there is something that we do now that is not reflected here that could be a bit confusing, which is indicate that the upstream-only commits are not incorporated into our working directory. In our current UI, upstream-only commits are above the uncommitted changes and everything those changes depend on (ie, are merged locally) are below that block.

We should indicate this somehow. here, you are interspersing the commits by time of commit, but I don't think that's nearly as important as wether it's merged into our working directory or it's remote only (ie, pending).

Perhaps we force all the upstream work to the top and have a dividing line, or even move them again above the uncommitted changes section. I'm not sure what works better, but they should be a removed block that is clearly unincorporated.

@krlvi
Copy link
Member Author

krlvi commented Mar 19, 2024

@schacon I was thinking that the red/orange color, together with the the absence of a merge of the two tracks indicates upstream only. Do you mean always keeping the upstream bit on the top to further highlight that like this mockup 👇 ?

image

I think one less nice thing in this arrangement is that newest commits (one created locally) wont be rendered on the top but maybe thats ok?

@schacon
Copy link
Member

schacon commented Mar 19, 2024

Exactly. I think since there's three colors, intermixing them would be confusing as to what is in your working directory and what is not. The upstream ones still need to be integrated and that should be clear. Also, we'll need a UI for that (merge/rebase these), which we're missing here. Those upstream commits are a really different animal and this interface makes them seem equivalent.

@krlvi
Copy link
Member Author

krlvi commented Mar 19, 2024

Also, we'll need a UI for that (merge/rebase these), which we're missing here.

wouldn't the button below cover those cases? it changes it's mode (merge upstream / [force]push) with an icon matching the color + highlighting the commits to be merged / pushed on hover

@schacon
Copy link
Member

schacon commented Mar 19, 2024

Oh sorry, I missed that. Yeah, that could work.

Though, can we do a mockup where we have "merge upstream" directly under the upstream group and "push to remote" under the local group?

Copy link
Member Author

krlvi commented Mar 19, 2024

I would also be interested in seeing that. But I wonder if we could somehow do it with buttons that are smaller than those big full-width ones, somehow it feels that they break the continuity / flow for me when they are in between other stuff (like in the current implementation)

@schacon
Copy link
Member

schacon commented Mar 19, 2024

Something like this (sorry for the horrible photoshop):

CleanShot 2024-03-19 at 16 28 29@2x

We should always group these together, and it seems a bit clearer that there is an action that is performed on that specific group. Also, there are then fewer dropdown options and we can keep the last one easily. Like, the top would be either "Merge" or "Rebase" and the user would probably always want that option. The locals would either be "push" or "open PR" (if no PR yet) and again would always want to default to that.

@schacon
Copy link
Member

schacon commented Mar 19, 2024

btw, I really like this where it is a single line, but it bends for the local ones:

CleanShot 2024-03-19 at 16 31 00@2x

It's a nice way to visually distinguish these. Also, we should remember that most of the time, this is all that will be there, there will much more rarely be upstream work.

@PavelLaptev
Copy link
Contributor

PavelLaptev commented Mar 19, 2024

Groups are cool, but instead of performing multiple actions on each group, let's focus on the most suitable action for the situation.

For instance, if there are upstream commits, we wouldn't recommend a "force push" (you also can't "push to remote" in this case) as the immediate action; instead, the preferable action would be to merge first. Let's explore using a dropdown menu for this purpose, similar to the concept in GitHub Desktop. It helps by suggesting the most common actions, making workflows smoother.

Let me make some new drafts 🙂 I'll try to make something today.

image image

@schacon
Copy link
Member

schacon commented Mar 19, 2024

Divergence in this case is the only correct way to show it. What might be valuable is to show a partial sha in these cases (or maybe even generally?)

@krlvi
Copy link
Member Author

krlvi commented Mar 19, 2024

Divergence in this case is the only correct way to show it. What might be valuable is to show a partial sha in these cases (or maybe even generally?)

In this case, if i have 10 commits that I have already pushed, and then choose to rebase my working directory, until I (force) push, I will have 20 commits on screen, with the upstream ones shown on top, right? I wonder if we can improve on that situation.

Btw, what is a partial sha?

@schacon
Copy link
Member

schacon commented Mar 19, 2024

maybe if we can match on identical message (or we could be spiffy and extract a patch-id from each, but there are corner cases that fuck both of those approaches - commit messages are generally fine and fast), we could do some viz where we collapse the rebased ones and mostly show the new ones. I think this is rather common, esp the more we add rebasing methods.

@schacon
Copy link
Member

schacon commented Mar 19, 2024

Btw, what is a partial sha?

Just showing the first 6-8 chars, rather than all 40 chars.

@krlvi
Copy link
Member Author

krlvi commented Mar 19, 2024

Here's a crazy idea - what if we use the base as an anchor for this. When a user rebases, we show the commits only once but with nodes in both tracks (remote and local) (illustration 3.).
We can visualise that there is divergency by:

  • on hover on the node we show the shas
  • the two tracks have different bases - the one that is local we can hover over, the remote branch has a line that extends further down (the base is somewhere off screen)

IMG_4231

What we could gain from this is simpler / more compact representation, and also communicate that it's not that the commit content is different but it's the base that actually differs.

WDYT?

@kiprasmel
Copy link

kiprasmel commented Mar 19, 2024

@krlvi are you re-discovering git range-diff by chance? 😅

see also https://github.com/kiprasmel/git-rebase-diff

@kiprasmel
Copy link

kiprasmel commented Mar 19, 2024

re: divergence, and re-matching / reconciling old <-> new commit SHAs: you might be interested in what git-stacked-rebase does:
https://github.com/kiprasmel/git-stacked-rebase/tree/nightly/git-reconcile-rewritten-list

tl;dr: upon performing a rebase, git writes a .git/rebase-merge/rewritten-list file with old->new SHA pairs.
but, the info is not complete, esp. when dealing with edit, break and exec statements in the interactive rebase form. (the statements pause the rebase and allow e.g. git commit --amend, git reset, git commit multiple times, which introduces incorrectness / incompleteness to the rewritten-list file).

the git-reconcile-rewritten-list tries to fix these edge-cases. it probably has some bugs yet to be ironed out, but usually it works.

@PavelLaptev
Copy link
Contributor

PavelLaptev commented Mar 24, 2024

Here's a crazy idea - what if we use the base as an anchor for this. When a user rebases, we show the commits only once but with nodes in both tracks (remote and local) (illustration 3.). We can visualise that there is divergency by:

  • on hover on the node we show the shas
  • the two tracks have different bases - the one that is local we can hover over, the remote branch has a line that extends further down (the base is somewhere off screen)

IMG_4231

What we could gain from this is simpler / more compact representation, and also communicate that it's not that the commit content is different but it's the base that actually differs.

WDYT?

@krlvi about no.3 - rebased. Is this concept introduce ability to only rebase, right?
Because right now we can only merge and rebase PRs, but not branches.

image

@krlvi
Copy link
Member Author

krlvi commented Mar 24, 2024

hmm, this is another rebase - it's when you have performed it and your branch is still in progress, while "rebase and merge" is an operation on completion

@PavelLaptev
Copy link
Contributor

@kiril got it. So, if you've merged and rebased the branch, it's considered integrated. Therefore, we should also have a "Rebase branch" option, for example, in this dropdown menu.

image

and then allow to select to which branch we want to rebase. e.g.

image

@krlvi
Copy link
Member Author

krlvi commented Mar 24, 2024

Let's discuss tomorrow, I sense there is a confusion here. But in brief, the Merge Pull Request, Rebase and Merge, Squash and merge operations that a person does on GitHub are shortcuts for the way the pull request ends up on master.

This is quite different, workflow wise, from rebasing while still working on the branch. For example our update button may perform a rebase.

In the screenshot above, the "Rebase branch" is not applicable since GitButler maintains the same base for all branches in the working directory. The red "update" button in the side/Navigation is the de-factor Rebase button

@PavelLaptev
Copy link
Contributor

PavelLaptev commented Mar 24, 2024

yes, I understand that Rebase and Merge, Squash and merge are shortcuts.

So, the update button is rebase but it rebases commits from on base commit to another, right? It's not like on the picture below?

image

@PavelLaptev
Copy link
Contributor

I misunderstood @krlvi. We discussed today what the rebase and merge means in terms of virtual branches. @krlvi thanks for visual representations of various corner cases 🖤. I'll share some drafts soon.

image

@PavelLaptev
Copy link
Contributor

Hey! Made new design drafts here:
https://www.figma.com/file/FbeLt0yjY9RiNn8MXUXsYs/Client-Design?type=design&node-id=2524%3A59821&mode=design&t=BU6UN2A1rkd006La-1


Here are flows you can find in drafts:

  • simple local changes push
  • merge upstream commits
  • rebase upstream commits
  • rebase whole workspace

@krlvi
Copy link
Member Author

krlvi commented Mar 26, 2024

Looking really nice @PavelLaptev! Looks accurate. I will leave some comments in figma on specific things where i had some thoughts. Also, is it worth making some kinda of design that shows the developer that part of the stuff is not on their computer. Not like the dotted line here #3121 (comment) but something 🤔

@PavelLaptev
Copy link
Contributor

@krlvi gotcha, thanks! I'll address your comments

@PavelLaptev
Copy link
Contributor

Another question is how we want to display unfolded commits.

Currently, we show it like this, in a single lane. Which works, but sometimes it feels a bit squeezed, so you need to resize the lane manually:

Screen.Recording.2024-03-27.at.14.48.45.mov

Another way is to show on the left, like this:

Screen.Recording.2024-03-27.at.14.52.10.mov

What do you think?

@krlvi
Copy link
Member Author

krlvi commented Mar 27, 2024

hmm, while the second option looks better, i feel that the double expand to the right makes it extra tricky to see the contents at a glance (an additional click and a long travel distance to get to the content of a file).

im worried that it's already a bit tricky to navigate/scroll left/right even for simpler things like moving hunks.
Are there more options that are possible other than those 2 that don't involve disruptive modals?

@PavelLaptev
Copy link
Contributor

Another option could be like this to open is aside, but anyway it will take horizontal space:

Screen.Recording.2024-03-27.at.15.12.46.mov

And another option where we open the commit card vertically, but show the hunk aside:

Screen.Recording.2024-03-27.at.15.30.38.mov

@PavelLaptev
Copy link
Contributor

PavelLaptev commented Mar 27, 2024

@krlvi I don't think that moving hunks related to this, because you can move hunks between commits. Now you can only sqash the whole commit card. I think that we can solve mooving hunks issue aside from this, I mean it's not the part of the problem of how narrow lines are. If you have 20 branches it will be a problem if you have very narrow branches and open everything vertically. The context menu option move file to… or move files to… could solve this

@PavelLaptev
Copy link
Contributor

PavelLaptev commented Mar 27, 2024

We continue to discuss this on Discord here https://discord.com/channels/1060193121130000425/1060193121666863156/1222544812998397962

@PavelLaptev
Copy link
Contributor

@krlvi
Copy link
Member Author

krlvi commented Jun 1, 2024

@krlvi krlvi closed this as completed Jun 1, 2024
@krlvi krlvi unpinned this issue Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement to an existing feature UX/UI Focusing on user satisfaction, usability, and overall experience
Projects
None yet
Development

No branches or pull requests

6 participants