Jakub Narebski [off-list ref] writes:
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.
--- Junio C Hamano <junkio@cox.net> wrote:
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.
Agree.
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
Agree.
Luben
Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
Changes:
[...]
quoted
* <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.
Emphasisis on "should". Check
http://roke(.)dyndns(.)info/cgi-bin/gitweb/gitweb.cgi?p=git.git;a=commitdiff;h=gitweb/test
By the way, should we use quoted or unquoted filename?
--
Jakub Narebski
Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
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.
Code is a bit horrible I think in trying to be smart and avoid
code repetition.
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).
Good idea. We have lost visible links with the change. And easy to
implement: just remove style override for .diff.header a.list from
CSS (or what would be better but harder to code remove "list" class
from <a> element in "git header").
Originally there was visible link with full sha1 of blob as text,
and hidden link with filename as text; perhaps dropping the latter
---/+++ line link is better idea (here hyperlink should I think be
hidden, if there is any, to match git-diff and GNU diff -u output).
And this would simplify code somewhat...
The hidden (but here I think we can change to not-hidden links) links
in the "index" extended header line are to stay I think: this is the
only place where we can have links to all the versions of file in the
future combined commitdiff format.
quoted
* 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.
Well, the old version had "(<file type>)" in gitweb diff header.
Not all git/gitweb users are familiar with POSIX numeric modes;
perhaps changing the style (color for example) of added stuff
to mark it as added would be enough?
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.
I used 7 hexdigits because git-diff uses 7 by default.
quoted
* <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.
I'm not sure if we shouldn't unquote filename always, or at least
remove double quotes surrounding filename, or [ab]/filename. The decision
to leave visible filename quoted was based on the troubles to code
it correctly in "git diff" header where one of the filenames might
not be hyperlinked: if file was added or deleted.
quoted
* from-file/to-file two-line header lines have slightly darker color
than removed/added lines.
This visually feels right.
What is left is fine-tuning the color.
quoted
* chunk header has now delicate line above for easier finding chunk
boundary, and top margin of 1px.
This visually feels right.
What is left is fine-tuning the line and vertical space.
--
Jakub Narebski
Luben Tuikov wrote:
quoted hunk
--- Jakub Narebski <jnareb@gmail.com> wrote:
quoted
By the way, should we use quoted or unquoted filename?
For dear life, I cannot understand *WTF* is this argument about
"quoted" and "unquoted". Can this stop please? PLEASE?
Unquoted, natural, normal.
Luben
P.S. I don't want to punish the good people who do not put \n or \r or whatnot
in their filenames, just for this one ... "person" who does. UTF8 is a different
story.
The filenames which don't need quoting are not quoted. Only filenames
which need quoting (which have LF ('\n'), TAB ('\t'), \ ('\\') and " (\"))
are quoted and surronded in quotes to mark as quoted.
The only thing that is questionable is quoting files which have \ and "
and no other strange names.
--
Jakub Narebski
Jakub Narebski wrote:
The filenames which don't need quoting are not quoted. Only filenames
which need quoting (which have LF ('\n'), TAB ('\t'), \ ('\\') and " (\"))
are quoted and surronded in quotes to mark as quoted.
By the way, I think that unquote subroutine in gitweb is broken.
I think I'll try for new patchser view to work for ordinary filenames
(perhaps with metacharacters in them), and only then try to debug
for filenames which need quoting.
--
Jakub Narebski
--- Jakub Narebski <jnareb@gmail.com> wrote:
Junio C Hamano wrote:
quoted
Jakub Narebski [off-list ref] writes:
quoted
Changes:
[...]
quoted
quoted
* <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.
Emphasisis on "should". Check
http://roke(.)dyndns(.)info/cgi-bin/gitweb/gitweb.cgi?p=git.git;a=commitdiff;h=gitweb/test
By the way, should we use quoted or unquoted filename?
For dear life, I cannot understand *WTF* is this argument about
"quoted" and "unquoted". Can this stop please? PLEASE?
Unquoted, natural, normal.
Luben
P.S. I don't want to punish the good people who do not put \n or \r or whatnot
in their filenames, just for this one ... "person" who does. UTF8 is a different
story.
Jakub Narebski wrote:
Junio C Hamano wrote:
quoted
Jakub Narebski [off-list ref] writes:
quoted
* 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.
Well, the old version had "(<file type>)" in gitweb diff header.
Not all git/gitweb users are familiar with POSIX numeric modes;
perhaps changing the style (color for example) of added stuff
to mark it as added would be enough?
Or we could use the following code:
<abbr title="100755/-rwxr-xr-x (file)">100755</abbr>
which gives explanation on mouse-over.
--
Jakub Narebski