Re: [PATCH] Add `git diff2`, a GNU diff workalike
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:42:54
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] Add `git diff2`, a GNU diff workalike · Johannes Schindelin <hidden>
- 2016-06-15 · [PATCH] Add `git diff2`, a GNU diff workalike · Johannes Schindelin <hidden>
Hi, On Fri, 16 Feb 2007, Johannes Schindelin wrote:
On Fri, 16 Feb 2007, Junio C Hamano wrote:quoted
Johannes Schindelin [off-list ref] writes:quoted
+ if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) + return error("file/directory conflict: %s, %s", name1, name2);If a/frotz is a file and b/frotz/nitfol is there, I do not think we show an error; we say "a/frotz" was removed (see notes below, though).Good point. Will fix.
Thinking about this again, I do not know of any patch implementation which removes a directory which just became empty, so dir->file is a real problem. Also, if dir is empty, another problem looms. So at least the dir->non-dir case should be an error. BTW I just realized that diff2 as-is will output a diff header for _every_ file pair, even if they do compare equally. Actually, I like it, so I don't want to imitate GNU diff behaviour here. Ciao, Dscho P.S.: Here is a quick-fix patch on top of my original (tested with dir->file (error!), file->dir, link->link, link->null, file->link): builtin-diff2.c | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/builtin-diff2.c b/builtin-diff2.c
index 1de82c1..234dabb 100644
--- a/builtin-diff2.c
+++ b/builtin-diff2.c@@ -15,7 +15,7 @@ static int read_directory(const char *path, struct path_list *list) while ((e = readdir(dir))) if (strcmp(".", e->d_name) && strcmp("..", e->d_name)) - path_list_insert(xstrdup(e->d_name), list); + path_list_insert(e->d_name, list); closedir(dir); return 0;
@@ -28,18 +28,24 @@ static int queue_diff(struct diff_options *o, int mode1 = 0, mode2 = 0; if (name1) { - if (stat(name1, &st)) + if (lstat(name1, &st)) return error("Could not access '%s'", name1); mode1 = st.st_mode; } if (name2) { - if (stat(name2, &st)) + if (lstat(name2, &st)) return error("Could not access '%s'", name1); mode2 = st.st_mode; } - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) - return error("file/directory conflict: %s, %s", name1, name2); + if (mode1 && mode2) { + if (S_ISDIR(mode1) && !S_ISDIR(mode2)) + return error("Cannot handle dir->file: %s -> %s", + name1, name2); + if (!S_ISDIR(mode1) && S_ISDIR(mode2)) + return queue_diff(o, name1, NULL) || + queue_diff(o, NULL, name2); + } if (S_ISDIR(mode1) || S_ISDIR(mode2)) { char buffer1[PATH_MAX], buffer2[PATH_MAX];