Re: [RFC] gitweb TODO

2 messages, 2 authors, 2016-08-11 · open the first message on its own page

Re: [RFC] gitweb TODO

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:35:36

Jakub Narebski [off-list ref] writes:
These are a few gitweb issues and features I'm currently working on 
(or plan working on).

1. New patchset view (commitdiff, blobdiff)

 c. "Cache" git diff header, or the whole patch, or the whole patchset.
    It is enough to cache (write lines to "buffer"/"cache" array) up to
    the extended header "^index" line, which can be used to check if to
    go to the next dofftree "raw" line (or wven which of "raw" difftree
    lines this particular patch corresponds to). Does not require
    changes in diff core, and is less fragile, less susceptible to
    breakage.
This is the most appropriate.  Right now it is not independently
controllable but it is not so inplausible for somebody to want
to get non recursive view of 'raw' part along with textual diffs
when running "--raw -p" diff and your solution c. is robust
against even such changes.

I would probably not call that "caching" but keeping track of
where you are, and you would need to know in which filepair you
are in anyway when you want to implement links to blob view from
the hunk header lines.
2. Difftree combined diff gitweb "raw" format
...
I have though about using one of the combined diff outputs for merge 
commits. The problem is how to represent the whatchanged part. Which 
parts of gitweb difftree output to leave? And what about the fact that 
we have raw output for -c/--combined diff format, but not for chunk 
simplifying --cc (compact combined) output?
I am not sure what's more useful to show there.  The part of the
output shows "the list of files that have changed by this
commit" for non-merge commits, so "the list of files that have
changed in this merge" is what you would want to show, but there
are three ways you can define "what changed" for a merge (see
the message that explains --cc to linux@horizontal I sent
yesterday).  I find "diff --name-status $it^1 $it" the most
natural semantics for the most of the time, and that is what we
do for --stat output.

If you want to treat all the parents equally, you could read
from "diff-tree -c --raw $it" which would show list of files
that needed file-level merges, along with the blob object names
from all parents.

We might want to give that when "diff-tree --cc --patch-with-raw
$it" is asked for in the "raw" part.  I dunno.
3. Committags support (and implementation)
...
Which of those would be better to implement?
I think that is a minor implementation detail; I think the
latter is probably less painful to maintain in the longer run
because if you encode things in earlier stages, postprocessing
stages need to know which part of the result from the earlier
processing needs decoding before further processing, but I
suspect you will be the one primarily hacking on that part, so
it is not my preference, but just a suggestion to make your life
less painful.

Re: [RFC] gitweb TODO

From: Jakub Narebski <hidden>
Date: 2016-08-11 20:41:20

Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
These are a few gitweb issues and features I'm currently working on 
(or plan working on).

1. New patchset view (commitdiff, blobdiff)

 c. "Cache" git diff header, or the whole patch, or the whole patchset.
    It is enough to cache (write lines to "buffer"/"cache" array) up to
    the extended header "^index" line, which can be used to check if to
    go to the next dofftree "raw" line (or wven which of "raw" difftree
    lines this particular patch corresponds to). Does not require
    changes in diff core, and is less fragile, less susceptible to
    breakage.
This is the most appropriate.  Right now it is not independently
controllable but it is not so inplausible for somebody to want
to get non recursive view of 'raw' part along with textual diffs
when running "--raw -p" diff and your solution c. is robust
against even such changes.
What about the fact that git-diff -M is _not_ patch-compatibile;
does creating two patches for one difftree raw format line for
mode change/'T' status (I guess only for "type" mode changes, i.e.
file to/from symlink, file to/from directory) helps understanding
the change? If not, perhaps it would be better to introduce option
to not break the patch...
 
I would probably not call that "caching" but keeping track of
where you are, and you would need to know in which filepair you
are in anyway when you want to implement links to blob view from
the hunk header lines.
I'd say "buffering" rather than "caching". The problem is that
you have to read up to the "index <hash>..<hash>[ <mode>]" to
check if you have to go to the next raw difftree line or not.
And the info from difftree line is needed to hyperlink parts
of extended git diff header (and also to hyperlink parts of
patch).
quoted
2. Difftree combined diff gitweb "raw" format
...
I have though about using one of the combined diff outputs for merge 
commits. The problem is how to represent the whatchanged part. Which 
parts of gitweb difftree output to leave? And what about the fact that 
we have raw output for -c/--combined diff format, but not for chunk 
simplifying --cc (compact combined) output?
I am not sure what's more useful to show there.  The part of the
output shows "the list of files that have changed by this
commit" for non-merge commits, so "the list of files that have
changed in this merge" is what you would want to show, but there
are three ways you can define "what changed" for a merge (see
the message that explains --cc to linux@horizontal I sent
yesterday).  I find "diff --name-status $it^1 $it" the most
natural semantics for the most of the time, and that is what we
do for --stat output.

If you want to treat all the parents equally, you could read
from "diff-tree -c --raw $it" which would show list of files
that needed file-level merges, along with the blob object names
from all parents.
We should have whatchanged part corresponding to the patchset
part at least in "commitdiff" view, which means '-c' (and for
the time being perhaps mean '-c' also in patchset part). '--cc'
which uses '-c' for the raw part would be nice...

"Commit" view could use "diff --name-status $hash^1 $hash", 
(i.e. I think the same what it does now), even if it is not
compatibile with "commitdiff" view.

-- 
Jakub Narebski
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help