🚀 Improve line number to make UI more like github

[kevinhwang91]

left side is github, right side is delta:

Here is my config about delta:

export GIT_PAGER="delta -n --syntax-theme=TwoDark -w 1 \
    --hunk-header-style='syntax bold' \
    --hunk-header-decoration-style='black box' \
    --file-style='yellow italic' \
    --file-decoration-style='yellow ul' \
    --minus-style='syntax #450a15' \
    --minus-emph-style='syntax #600818' \
    --plus-style='syntax #0b4820' \
    --plus-emph-style='syntax #175c2e' \
    --line-numbers-zero-style='#4b5263' \
    --line-numbers-minus-style='syntax #600818' \
    --line-numbers-plus-style='syntax #0b4820' \
    --line-numbers-left-format='{nm:^4} ' --line-numbers-right-format='{np:^4}  '"

From the above image, UI of delta about line number is so weird.

[dandavison]

Hi @kevinhwang91, yes you're right, the background colors aren't making much sense currently :/ Thanks for this! I'll have another think about this and modify the behavior (without changing the interface hopefully) so that the background colors form contiguous blocks in this situation.

[bew]

Hello, I'd be interested in this as well!
How is your thinking on how to do this?
(I'd like to try to make a PR if you think it's good as a first contribution)

[dandavison]

Hi @bew I'd be happy for someone to make this possible. I don't have time to look into it currently unfortunately. IIRC the current configuration interface doesn't quite make this possible. I would suggest:

• Step 1: Play around with the various line-numbers-*-style options to convince yourself that it is/isn't possible today.

• Step 2: Identify a way of making it possible, ideally without changing the meaning of current options, so that other user's configs are not broken. Alternatively, perhaps you identify some change to the options that does change the meaning, and you present an argument that other users are likely to like the new meaning more than the old!

[danwilliams]

I stumbled upon this problem today 🙂 I notice this issue is quite old - has anyone done any work on it? (@dandavison @bew) If not, I'll see if I can find some time to fix it.

[bew]

@dandavison go for it, I got caught up with other configs and never went back to it (although I was thinking about it yesterday for some reason 🤷‍♂️)

[dandavison]

@danwilliams yes go for it, I haven't been able to get to it. Could be helpful to post a design proposal (i.e. any modified meanings of existing options plus any new options) before doing too much implementation. Bear in mind that it needs to all make sense with and without side-by-side mode active, and that the configuration options are already quite complex.

[danwilliams]

Okay, cool 👍

[danwilliams]

@dandavison @bew it turned out to be extremely simple - once I'd identified how the code worked, which only took a few minutes due to it being very well structured, the change was simple and obvious.

@dandavison the tests are a little odd - they work but the feedback does not make it overly clear exactly which bit is failing. So that took longer to grok than the code fix 😄

I have added some screenshots to the PR - it seems much better now and behaves as I would expect. However, please beware that I've only tested the things I am aware of that are relevant and affected by this change. In particular I don't think any documentation updates are necessary, as I see this as a bugfix more than anything, but if you disagree then please direct me to whichever area of documentation you feel might need to have something written about this 🙂

[danwilliams]

@kevinhwang91 @bew I'm curious if my PR fixes the behaviour in your opinions? 🙂