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

See divergence from base branch #3536

Open
stefanhaller opened this issue Apr 28, 2024 · 7 comments
Open

See divergence from base branch #3536

stefanhaller opened this issue Apr 28, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@stefanhaller
Copy link
Collaborator

stefanhaller commented Apr 28, 2024

For the feature branch I'm on, I want to see how it diverges from the main branch that it was branched off of (we'll call this the "base branch"). In particular, I want to see how far it has fallen behind, to get a feeling for how urgent it is to rebase onto master again, or how hard that might be.

This issue was inspired by a workflow that some people use (see #3437), where the feature branch's upstream is set to origin/master; this way you see the divergence from master both in the ↑m↓n display of the branches list, and in the divergence log that you get with u enter. This needs to be combined with git config push.default current, so that pushing the branch goes to origin/feature rather than origin/master. This workflow is only useful for people working alone on their branch, and only from a single machine, so that they don't have to care about divergence from their origin/feature branch. (Another property of that workflow is that pressing p rebases onto origin/master; I care less about this one, and it's not part of this issue.)

I want to make the same information available for people working "normally", e.g. where a branch's upstream is set to origin/feature. This has two parts:

  1. Making the divergence information permanently visible in the branches list, similar to the ↑m↓n that we already display.
  2. Getting an on-demand divergence log similar to the one we show for u enter.

I find 1. more desirable to have, but it's also harder in many ways, so I'll start by discussing 2.

2. On-demand divergence log

UI design

Pretty simple: we add a command "View divergence from base branch" to the upstream menu, shortcut b. I find it important that I don't have to tell lazygit which of my main branches that is, it should figure this out automatically.

It is not clear whether we need to show a full divergence log; what's really interesting here is only the "Remote" part of it. The "Local" part is always the same as the non-green commits in the commits panel. However, I'd still vote for showing the full divergence log, for consistency with the divergence against upstream, and because it makes it clearer what it means.

Implementation

The main challenge is how to determine the base branch (i.e. the closest main branch). I think this can be done as follows:

<merge-base-hash> = git merge-base HEAD <all-existing-main-branches...>
result = git for-each-ref --contains <merge-base-hash> <all-existing-main-branches...> | head -1

The second command will only return more than one result if the feature branch was branched off before the point where one main branch was branched off another. In this case it's impossible to determine which of the two main branches is supposed to be considered the "base" for the feature branch, and it's also not a common case in practice, so we arbitrarily pick the first one.

We already have code to determine the existing main branches, and cache them, but it's done privately inside CommitLoader right now. We need to extract this to a more common place (and protect it with a mutex, probably), and store the value somewhere in the model.

The two commands together take well under 100ms to execute on my machine, so it seems we can simply invoke them every time the command is issued.

1. Show permanently in branches list

UI design

This is tricky. I can't find a good way to visualize the information so that it's easy to understand but not too noisy. What I tried:

  • show a second pair of ↑m↓n after the one for the upstream branch, in a different color (cyan seems to work best). This is way too noisy, completely unusable. Also, the ↑m part is not really very interesting, it's just the length of the branch.
  • show just ↓n in cyan behind the information for the upstream branch. Omit if n is zero. This works a little better, but still seems too noisy. But maybe I could get used to it, and we could make it optional.

Another idea could be to provide a keybinding that toggles between the normal upstream information and the divergence-from-base-branch information. We'd have to indicate in the title bar which mode we're in (similar to ignoring whitespace in diffs); then again, if we show it in a different color, maybe that's enough.

Implementation

This is also a little more difficult. It seems we have to compute the divergence for each local branch separately by first determining the base branch as in 2., and then doing git rev-list --left-right --count HEAD...<main-branch> to determine the divergence counts. This will have to be done in the background, similar to how we determine the recency information, so as a user you'll see these trickle in with a bit of delay.

Next steps

I'll probably start by implementing 2., as it's relatively clear, and already provides some value.

For 1., I'd like to get input mostly on the UI design question.

Ping @jesseduffield in case you don't get notified of new issues automatically.

@stefanhaller stefanhaller added the enhancement New feature or request label Apr 28, 2024
stefanhaller added a commit that referenced this issue Apr 30, 2024
- **PR Description**

In the "View divergence from upstream" view we have so far disabled the
commit graph because it was too difficult to implement properly. I
really miss it though, so here's a PR that enables it there, too.

For feature branches it is not essential, because these usually don't
contain merges and the graph is a trivial line. However, for the master
branch against its upstream it is useful too see how many PRs were
merged since you last fetched it, and the graph helps a lot with that.
Also, when we implement #3536 it will be very useful there, too.
@stefanhaller stefanhaller changed the title See divergence from closest main branch See divergence from base branch May 3, 2024
@stefanhaller
Copy link
Collaborator Author

I hacked on this a bit, and I came up with something that I already don't want to miss any more:

image

Yes, it's very noisy, but it's also so useful that I think it's totally worth it, at least for me. I'm happy to make it optional, but I'm pretty sure I would leave it on, always.

Of course this will look a lot worse if you have a large number of stale branches. In one of my work repos I have some 50 local branches or more, and many of them I haven't touched for years, so they have fallen behind their base branch by a 5- or even 6-figure number of commits. It doesn't bother me though, on the contrary: it gives me a better sense of how old my branches are than the 8M age display on the left.

@jesseduffield Very keen to hear you thoughts on all this before I spend more time on it.

@jesseduffield
Copy link
Owner

I like what I'm seeing in that screenshot: the use of blue makes it less noisy than it otherwise would be.

It's also less noisy than what I had assumed was a solution to the problem which is having commit hashes against each branch with the green/yellow/red colouring. I believe showing the ahead/behind count basically gives us all the same information plus more (i.e. 'is this branch fully merged into the base branch so I can delete it', 'do I need to rebase onto the base branch').

Pending testing this feature out myself I'd be happy to make it appear by default.

@stefanhaller
Copy link
Collaborator Author

(i.e. 'is this branch fully merged into the base branch so I can delete it', 'do I need to rebase onto the base branch').

In the current state of my hack it only gives you the second of these, not the first. In order to have the first as well, we'd have to show both the ahead and behind counts; currently I only show behind, to reduce the noise.

Showing both would look like this:

image

I admit that the ahead information is also useful, not only to tell whether the branch is merged already, but also to tell how long each branch is, which could be useful to know. However, this is really becoming so noisy that the usefulness doesn't outweigh it for my taste.

And btw, the indication of whether the branch is fully merged wouldn't always be accurate, anyway: if a maintainer rebases and force-pushes your branch in order to merge it (like we often do here), then your local branch would still show as unmerged until you pull it.

Later today I'll try to clean up my branch enough so that I can dare post it as a draft PR for testing.

@stefanhaller
Copy link
Collaborator Author

OK, no draft PR yet, but a branch to try out if you want. I have a stack of three branches, and it's probably going to be three PRs in the end, but rebase-onto-base-branch is the top one if you want to test the whole package. It contains the display in the branches list, the "View divergence from base branch" command in the upstream menu, and the "Rebase onto base branch" command in the rebase menu.

For the display, I introduced a config that lets you choose between showing none, only the behind count, or behind and ahead counts. This is only for easier testing for now, I'm not sure yet if we will have a config in the end.

@jesseduffield
Copy link
Owner

@stefanhaller nice. It's definitely noisy. I'll give it a go for a couple days to see how I adjust.

As for the divergence keybinding: I don't think that belongs in the upstream menu, because iirc the base branch is not necessarily a remote branch. What do you think about having this live in the diffing menu? We can also have the other divergence options live there (in addition to where it currently lives)

@stefanhaller
Copy link
Collaborator Author

I don't think that belongs in the upstream menu. What do you think about having this live in the diffing menu?

Interesting, we already discussed this back when I added the divergence from upstream command. My opinion from back then still holds for this one too. Also, currently the diffing menu only contains commands that enter diffing mode, so this one would be out of place there for this reason alone.

As for the upstream menu: you are right that the base branch is not technically the upstream in the narrow technical sense in which git uses the term (i.e. feature@{upstream}). However, you could still argue that conceptually, the base branch is the next level of "upstream" after the remote tracking branch, so for me it still makes some sense to put it there.

@jesseduffield
Copy link
Owner

I vaguely recalled that we may have discussed this before. I'm happy to go with that and see how users find it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants