Problem/Motivation
In #2790837: Introduce gitHub Layout plugin we have added a single column layout for diff comparison, a good improvement for it can be to add line numbers to the diff display.
Proposed resolution
Add line numbers, add a patch and provide tests.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | Screenshot from 2016-09-16 09-29-35.png | 30.07 KB | johnchque |
| #14 | interdiff-2793029-11-14.txt | 728 bytes | johnchque |
| #14 | add_line_numbers_to-2793029-14.patch | 5.67 KB | johnchque |
| #11 | interdiff-2793029-9-11.txt | 3.38 KB | johnchque |
| #11 | add_line_numbers_to-2793029-11.patch | 5.07 KB | johnchque |
Comments
Comment #2
johnchqueAdded related issue.
Comment #3
miro_dietikerYeah that would be a cool thing. But i think bringing in the visual plugin is way more prio.
BTW plz i think we should have some second issue for line numbers of the two col layout.
We might offer a plugin setting to enable / disable line numbers.
But also unsure if we will offer to the end user to enable this when watching a diff..?
Comment #4
miro_dietikerOh, this needs new columns... That's going to move lots of things.
Also, newly filters that strip tags are changing line numbers, making the references wrong.
I guess it means we should only display line numbers for the unfiltered processing.
If we touch the cols, we should also check #2785857: Introduce header / row table keys
Comment #5
johnchqueThe attached patch adds lines like this:
Comment #7
miro_dietikerGitHub adds two line columns on the merged / unified / single column view, one per side.
And the UI above reads like lines are falling apart. IMHO we have way too much spaces between lines still.
The GitHub output reads much more continuous.
You only output numbers below 10. Are these properly right aligned each?
I think i would prefer to see line numbers with a fixed width font.
This also applies to all the content in the raw values for me, like GitHub does.
Not sure if we want to stay with a variable width font for stripped content?
Comment #8
miro_dietikerAh one thing we shouldn't forget:
GitHub also marks the full row with background color. We also should do this IMHO.
Comment #9
johnchqueTrue, looks much better IMHO!
Comment #11
johnchqueTests fixed. Added few tests too.
Comment #13
miro_dietikerI also think
- we could grey the line number a bit as they are not primary information.
- and i consider adding those vertical lines with line numbers.
Eager to get this in! :-)
Comment #14
johnchqueYes, looks even nicer.
Comment #15
miro_dietikerWohoo, this looks like a REAL diff! ;-)
Committed it is!
(Needed a rebase as the plugin was meanwhile renamed...)