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

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

Added related issue.

miro_dietiker’s picture

Yeah 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..?

miro_dietiker’s picture

Oh, 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

johnchque’s picture

Status: Active » Needs review
StatusFileSize
new1.49 KB
new40.24 KB

The attached patch adds lines like this:

Status: Needs review » Needs work

The last submitted patch, 5: add_line_numbers_to-2793029-5.patch, failed testing.

miro_dietiker’s picture

Issue summary: View changes
StatusFileSize
new35.55 KB

GitHub 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?

GitHub example

miro_dietiker’s picture

Ah one thing we shouldn't forget:
GitHub also marks the full row with background color. We also should do this IMHO.

johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new2.91 KB
new2.97 KB
new35.28 KB

True, looks much better IMHO!

Status: Needs review » Needs work

The last submitted patch, 9: add_line_numbers_to-2793029-9.patch, failed testing.

johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new5.07 KB
new3.38 KB

Tests fixed. Added few tests too.

Status: Needs review » Needs work

The last submitted patch, 11: add_line_numbers_to-2793029-11.patch, failed testing.

miro_dietiker’s picture

I 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! :-)

johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new5.67 KB
new728 bytes
new30.07 KB

Yes, looks even nicer.

miro_dietiker’s picture

Status: Needs review » Fixed

Wohoo, this looks like a REAL diff! ;-)

Committed it is!
(Needed a rebase as the plugin was meanwhile renamed...)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.