Re: [PATCH/RFC] gitweb: New improved patchset view

From: Junio C Hamano <junkio@cox.net>
Date: Oct 29, 2006, 11:47 UTC
Message-ID: <7viri34a3k.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200610291122.30852.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 9 quoted lines

> Changes:
> * "gitweb diff header" which looked for example like below:
>     file:_<sha1 before>_ -> file:_<sha1 after>_
>   where 'file' is file type and '<sha1>' is full sha1 of blob, is link
>   and uses default link style is changed to
>     diff --git a/<file before> b/<file after>
>   where <file> is hidden link (i.e. underline on hover, only)
>   to appropriate version of file. If file is added, a/<file> is not
>   hyperlinked, if file is deleted, b/<file> is not hyperlinked.

I do not have time to look at the code right now, but here are quick comments on the output.

I personally do not mind "hidden" but it might be more obvious to make them normal links -- the filenames in the header are not part of long text people need to "read". On the other hand, it feels a bit wasteful to have these hidden links both on "diff --git" line *and* ---/+++ lines (these three are very close to each other).

> * there is added "extended diff header", with <path> and <hash>
>   hyperlinked (and <hash> shortened to 7 characters), and <mode>
>   explained: '<mode>' is extnded to '<mode>/<symbolic mode> (<file type>)'.

It somehow feels that deviating from what "git diff" gives makes it very distracting; I would feel better if "/-rw-r--r-- (file)" were not there.

Also I think arguing over 7 or 8 hexdigits is pointless; if you are reading this from "git diff", it is probably the easiest to match what "git diff" gave you. One thing we _might_ want to do in the future is to change "git diff" to use DEFAULT_ABBREV hexdigits at the minimum but more if needed to disambiguate; I think it currently does not do the "more if needed" part.

> * <file> hyperlinking should work also when <file> is originally
>   quoted. For now we present filename quoted. This needed changes to
>   parse_difftree_raw_line subroutine.

This feels Ok.

> * from-file/to-file two-line header lines have slightly darker color
>   than removed/added lines.

This visually feels right.

> * chunk header has now delicate line above for easier finding chunk
>   boundary, and top margin of 1px.

This visually feels right.