Thread (17 messages) flat view 17 messages, 4 authors, 2016-08-11

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

From: Luben Tuikov <hidden>
Date: 2016-08-11 19:29:18

--- Jakub Narebski <jnareb@gmail.com> wrote:
A couple of questions regarding new patchset/diff look for gitweb.
Currently patch starts with "git diff" header

  diff --git a/file1 b/file2

then we have a couple of extended header lines

  old|new|deleted file|new file mode <mode>
  copy|rename from|to <path>
  (dis)?similarity index <percent>
  index <hash>..<hash> [<mode>]

then we have two-line from-file/to-file header
First revert back to monospace in the commit message.
As I mentioned in that other email, where the person
didn't CC you and thus so I didn't, see commit 
4b7ce6e2d6ba088da50de1df38b040ea2c0b8f18.

Can you please hyperlink what you had intended to
quote?  I.e. filenames which can lead the user browsing to a
state (pre-rename, post-rename, etc).  Thanks.
  --- a/file1
  +++ b/file2

then patch itself.


1. Which parts to convert to hyperlinks, and which to do not? Which
   links have visible and which hidden (underline on mouseover, the same
   color as neighbouring text)?
I think the current state of your patch is good, sans the comments
received so far: monospace, that bug in the index links, etc.
Lets get that in, and then you can RFC another improvement on top of this.
   I think that a/file1 in "git diff" header should be turned into
   visible hyperlink unless file is created, and b/file2 should be link
   unless file is deleted.
This is intuitive and makes sense.
   Also both <hash>-es in "index" extended header lines should be turned
   into links, as it is the only way to have hyperlink to all previou
Fix the bug though.
   versions of the file in the case of "combined diff" format (to be
   added later). The question is if those hyperlinks should be visible;
I personally love little hidden gems, but some people like everything
to be overly obvious to them.  Hidden gems are part of the learning,
but I'm sure I'm not going to convince everyone.

Hidden.
   I don't have compelling reason against. Should we use title attribute
   to give filename perhaps, or is it unnecessary?

   Currently file1 and file2 in "--- a/file1" and "+++ b/file2" are
   turned into hidden links. Should we leave it, or should we remove
   this link as we have similar link just above? If we decide to have
Leave it -- it's cool.
   this link, should we also hyperlink <path> in "copy|rename" extended
   header line?
If it makes sense.
2. Use quoted or unquoted filename, remove or leave surrounding quotes
   in quoted filename? Should we unquote the not hyperlinked filename
Are you familiar with the term "legalism"?
   in the case of creation/deletion? What should be span of link:
     "a/_file1_", "_a/file1_", _"a/file1"_
     "_file_", _"file"_ 
   (where '_' marks beginning and end of link) for quoted filenames?
   What should be span of link for unquoted filenames:
      a/_file_, _a/file_
   Currently gitweb uses a/_file_ in ---/+++ line.
Unquoted!  Now that we've solved this "problem", lets move on to more
interesting things. ;-)
3. How (and if) to explain numerical mode: the currently used
Let's not explain it for now.  Let's have your patch go in sans the
comments already posted.  We can always debate on that later.

Thanks,
   Luben

   <mode>/<symbolic mode> (<file type>) e.g. 100755/-rwxr-xr-x (file).
   Or <mode> (<file type>) should be enough? Should we mark the addition
   compared to git-diff output? Or should we explain <mode> only on
   mouseover, using for example:
     <abbr title="100644/-rw-r--r-- (file)">100644</abbr>
   or just
     <abbr title="executable file">100755</abbr>?
-- 
Jakub Narebski
Poland
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help