-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Comments
- **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.
I hacked on this a bit, and I came up with something that I already don't want to miss any more: 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 @jesseduffield Very keen to hear you thoughts on all this before I spend more time on it. |
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. |
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 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. |
@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) |
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. |
I vaguely recalled that we may have discussed this before. I'm happy to go with that and see how users find it |
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 withu enter
. This needs to be combined withgit 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 pressingp
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:
↑m↓n
that we already display.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:
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:
↑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.↓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.
The text was updated successfully, but these errors were encountered: