discussion on issue #77 (review icon position for in-line code comments)

[Mads Kiilerich]

On 01/26/2015 02:41 AM, Sean Farley wrote:
> Mads Kiilerich writes:
>> I also think it is odd that both Bitbucket, Github and Kallithea shows
>> the line numbers for each line in a diff. It is usually worthless
>> information when reviewing through the web. If using the actual source
>> for reference and reviewing locally, the line number of the chunk is
>> more releant than the line number of every line in the diff. I thus
>> think we could make an even better solution if we didn't use horizontal
>> space for both showing line numbers for every line and also leaving
>> space for 'add comment' and 'link to this line' (and in the future
>> perhaps also for annotate information). Perhaps by overlaying the latter
>> on top of the first when hovering over a line.
> Ok, this paragraph lost me. I'm guessing you agree to "have a way to
> link to a hunk"

yes ... and also for linking to a specific line. But we do not 
necessarily have to use screen space to all the time show a link for 
every line of code we show on the screen.

>   but posit that "the line number might not make sense
> (especially on the web)"?

Yes, as long as I can link to exactly the right line, it doesn't matter 
much what the actual number is.

(I guess the main use case for showing line numbers next to every line 
is when doing pair programming / reviewing using the same screen and 
wanting to be able to talk about lines without pointing. When using a 
review tool you usually have to point and click at the line anyway - 
just quoting its number is a bad hyper reference.)

> Why wouldn't the line numbers in the first column not match with the
> line numbers in the previous version of the file? Same for the second
> column and the diff'ed version of the file?

I don't know why they wouldn't (and I don't see how that question is 
related to what I said). (But one common reason for that is that you are 
looking at a slightly different version of the file.)

My point is just that it is a bit inefficient to use screen space to 
show a lot of line numbers where each one just one bigger than the 
previous. There is not a lot of information in that. Nobody needs an 
overview of line numbers like they need an overview of code. You know 
when you are looking for a line number, and you always do it like "now I 
want to know the line number of this line". Given that we have limited 
screen space and want to use it efficiently, I think we should consider 
if it could make sense to show something else instead of the line numbers.

> I'm all for displaying more information (like you mention with annotate)
> ESPECIALLY for a way to click back through history for that given hunk /
> section of a file. That's getting off-topic for this discussion, though.

It is still about how we can make the diff view "right" so all the 
relevant information and functionality is available and easy to use. We 
should avoid making micro optimizations if it doesn't help the bigger 
picture or conflicts with it.

/Mads