[RFC/PATCH] align D/F handling of "diff --no-index" with that of normal Git
When a commit changes a path P that used to be a file to a directory and create a new path P/X in it, "git show" would say that file P was removed and file P/X was created for such a commit. However, if we compare two directories, D1 and D2, where D1 has a file D1/P in it and D2 has a directory D2/P under which there is a file D2/P/X, and ask "git diff --no-index D1 D2" to show their differences, we simply get a refusal "file/directory conflict". The "diff --no-index" implementation has an underlying machinery that can make it more in line with the normal Git if it wanted to, but somehow it is not being exercised. The only thing we need to do, when we see a file P and a directory P/ (or the other way around) is to show the removal of a file P and then pretend as if we are comparing nothing with a whole directory P/, as the code is fully prepared to express a creation of everything in a directory (and if the comparison is between a directory P/ and a file P, then show the creation of the file and then let the existing code remove everything in P/). Signed-off-by: Junio C Hamano --- diff-no-index.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 265709b..52e9546 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o, if (get_mode(name1, &mode1) || get_mode(name2, &mode2)) return -1; - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) - return error("file/directory conflict: %s, %s", name1, name2); + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) { + struct diff_filespec *d1, *d2; + + if (S_ISDIR(mode1)) { + /* 2 is file that is created */ + d1 = noindex_filespec(NULL, 0); + d2 = noindex_filespec(name2, mode2); + name2 = NULL; + mode2 = 0; + } else { + /* 1 is file that is deleted */ + d1 = noindex_filespec(name1, mode2); + d2 = noindex_filespec(NULL, 0); + name1 = NULL; + mode1 = 0; + } + /* emit that file */ + diff_queue(&diff_queued_diff, d1, d2); + + /* and then let the entire directory created or deleted */ + } if (S_ISDIR(mode1) || S_ISDIR(mode2)) { struct strbuf buffer1 = STRBUF_INIT; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] align D/F handling of "diff --no-index" with that of normal Git
On Sun, Mar 22, 2015 at 1:11 AM, Junio C Hamano wrote: > When a commit changes a path P that used to be a file to a directory > and create a new path P/X in it, "git show" would say that file P s/create/creates/ More below... > was removed and file P/X was created for such a commit. > > However, if we compare two directories, D1 and D2, where D1 has a > file D1/P in it and D2 has a directory D2/P under which there is a > file D2/P/X, and ask "git diff --no-index D1 D2" to show their > differences, we simply get a refusal "file/directory conflict". > > The "diff --no-index" implementation has an underlying machinery > that can make it more in line with the normal Git if it wanted to, > but somehow it is not being exercised. The only thing we need to > do, when we see a file P and a directory P/ (or the other way > around) is to show the removal of a file P and then pretend as if we > are comparing nothing with a whole directory P/, as the code is > fully prepared to express a creation of everything in a directory > (and if the comparison is between a directory P/ and a file P, then > show the creation of the file and then let the existing code remove > everything in P/). > > Signed-off-by: Junio C Hamano > --- > diff-no-index.c | 23 +-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 265709b..52e9546 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o, > if (get_mode(name1, &mode1) || get_mode(name2, &mode2)) > return -1; > > - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) > - return error("file/directory conflict: %s, %s", name1, name2); > + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) { > + struct diff_filespec *d1, *d2; > + > + if (S_ISDIR(mode1)) { > + /* 2 is file that is created */ > + d1 = noindex_filespec(NULL, 0); > + d2 = noindex_filespec(name2, mode2); > + name2 = NULL; > + mode2 = 0; > + } else { > + /* 1 is file that is deleted */ > + d1 = noindex_filespec(name1, mode2); > + d2 = noindex_filespec(NULL, 0); > + name1 = NULL; > + mode1 = 0; > + } > + /* emit that file */ > + diff_queue(&diff_queued_diff, d1, d2); > + > + /* and then let the entire directory created or deleted */ s/created/be created/ > + } > > if (S_ISDIR(mode1) || S_ISDIR(mode2)) { > struct strbuf buffer1 = STRBUF_INIT; > -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] align D/F handling of "diff --no-index" with that of normal Git
On 22/03/15 05:11, Junio C Hamano wrote: > When a commit changes a path P that used to be a file to a directory > and create a new path P/X in it, "git show" would say that file P > was removed and file P/X was created for such a commit. > > However, if we compare two directories, D1 and D2, where D1 has a > file D1/P in it and D2 has a directory D2/P under which there is a > file D2/P/X, and ask "git diff --no-index D1 D2" to show their > differences, we simply get a refusal "file/directory conflict". > > The "diff --no-index" implementation has an underlying machinery > that can make it more in line with the normal Git if it wanted to, > but somehow it is not being exercised. The only thing we need to > do, when we see a file P and a directory P/ (or the other way > around) is to show the removal of a file P and then pretend as if we > are comparing nothing with a whole directory P/, as the code is > fully prepared to express a creation of everything in a directory > (and if the comparison is between a directory P/ and a file P, then > show the creation of the file and then let the existing code remove > everything in P/). > > Signed-off-by: Junio C Hamano > --- > diff-no-index.c | 23 +-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 265709b..52e9546 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o, > if (get_mode(name1, &mode1) || get_mode(name2, &mode2)) > return -1; > > - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) > - return error("file/directory conflict: %s, %s", name1, name2); > + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) { > + struct diff_filespec *d1, *d2; > + > + if (S_ISDIR(mode1)) { > + /* 2 is file that is created */ > + d1 = noindex_filespec(NULL, 0); > + d2 = noindex_filespec(name2, mode2); > + name2 = NULL; > + mode2 = 0; > + } else { > + /* 1 is file that is deleted */ > + d1 = noindex_filespec(name1, mode2); -^ I have not been following the discussion (or even really studied this patch), but the asymmetry here caught my eye as I was skimming the email. [So, if this is actually correct, sorry for the noise!] ATB, Ramsay Jones > + d2 = noindex_filespec(NULL, 0); > + name1 = NULL; > + mode1 = 0; > + } > + /* emit that file */ > + diff_queue(&diff_queued_diff, d1, d2); > + > + /* and then let the entire directory created or deleted */ > + } > > if (S_ISDIR(mode1) || S_ISDIR(mode2)) { > struct strbuf buffer1 = STRBUF_INIT; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] align D/F handling of "diff --no-index" with that of normal Git
On Sun, Mar 22, 2015 at 5:37 AM, Ramsay Jones wrote: > On 22/03/15 05:11, Junio C Hamano wrote: >> + if (S_ISDIR(mode1)) { >> + /* 2 is file that is created */ >> + d1 = noindex_filespec(NULL, 0); >> + d2 = noindex_filespec(name2, mode2); >> + name2 = NULL; >> + mode2 = 0; >> + } else { >> + /* 1 is file that is deleted */ >> + d1 = noindex_filespec(name1, mode2); > > I have not been following the discussion (or even really > studied this patch), but the asymmetry here caught my eye > as I was skimming the email. Yes, the assymetry is a stupid cut-and-paste error. Thanks for catching. There was actually no discussion on this point other than a tangential mention of the problem to be solved, so you are up to date as long as you read the proposed log message in the message you are responding to ;-) Another thing I noticed while I was playing with this is that when comparing D1 and D2, a path that was modified is shown as a patch between a/D1/path and b/D2/path, but a path that was created or removed shows D1 or D2 on both sides of the comparison, which we may also want to fix. This problem appears in the current codebase without the patch under discussion. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html