Enable the commit graph in the divergence view

[stefanhaller]

• 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.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.03% (target: -2.00%)	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (00c55d5)	50670	42639	84.15%
Head commit (b767357)	50718 (+48)	42693 (+54)	84.18% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3537)	68	68	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy will stop sending the deprecated coverage status from June 5th, 2024. Learn more}

[stefanhaller]

I added tests, and I now feel good enough about this to put it in review.

I experimented with simplifying the implementation by using lo.PartitionBy to partition the commits slice into sections; this does get rid of some "if" statements and makes the code shorter overall, but it also makes it much harder to understand (for my taste). Other suggestions welcome.

[jesseduffield]

LGTM, great work

[jesseduffield]

should this say 'we have some remote commits'?

[stefanhaller]

Happy to change it (see 440022a), but I'd be curious what you think was wrong with my version. Maybe there's something I don't understand about how English works? (What I meant to say was "this is the section of code that deals with the remote commits, so let's first check if we have any at all.")

[jesseduffield]

So I hadn't actually thought very hard about it until just now, but reading up here: https://dictionary.cambridge.org/grammar/british-grammar/at-all

At all means ‘in any way’. We use it with questions and negatives to add emphasis, but not with affirmative statements

I think 'let's check if we have any at all' sounds fine because it implies the question 'are there any at all?' but if you have a statement 'yes, we have any at all' it feels wrong.

My understanding is that you've said 'at all' to add emphasis because it would be especially silly for us to execute all this code that depends on there being remote commits if there are no remote commits. Seems like an arbitrary shortcoming of English: I can imagine an alternative history where you could use 'at all' in affirmative statements and there wouldn't be any asymmetry but it seems like for some strange reason that's the convention.

[stefanhaller]

Good to know, thanks for the explanation.

[jesseduffield]

No worries. Out of curiosity I've googled some more and this post sheds some more light on it: https://english.stackexchange.com/questions/63718/at-all-vs-not-at-all-in-negated-sentences

It's really fascinating to come across these situations where I'm like a neural net that's been trained on all this data without actually comprehending the underlying rules and now I'm looking at the actual structure of the thing that I've only had a fuzzy intuition on. Strange feeling haha

[jesseduffield]

likewise here

[jesseduffield]

I experimented with simplifying the implementation by using lo.PartitionBy to partition the commits slice into sections; this does get rid of some "if" statements and makes the code shorter overall, but it also makes it much harder to understand (for my taste). Other suggestions welcome.

I'm happy with the current approach