Changes since v6:
- avoid returning -1 to cmd_main() by adjusting the return site in
"create a tmpdir path without repeated slashes".
- "refactor dir-diff to write files using helper functions" was
reworked to add two helper functions instead of one so that the
common checks for *entry->{left,right} can be handled in a single place.
- write_entry() was renamed to write_file_in_directory() and its
signature was adjusted to match how write_file() takes its parameters.
- write_file_in_directory() gets called from the newly added
write_standin_files() helper which encompases the guts of
the symlinks and submodules hashmap loops.
- Comments were added describing the purpose of the helper functions.
David Aguilar (4):
difftool: create a tmpdir path without repeated slashes
difftool: refactor dir-diff to write files using helper functions
difftool: remove an unnecessary call to strbuf_release()
difftool: add a missing space to the run_dir_diff() comments
builtin/difftool.c | 104 ++++++++++++++++++++++----------------------
t/t7800-difftool.sh | 7 +++
2 files changed, 60 insertions(+), 51 deletions(-)
Range-diff against v6:
1: 121186ca0f ! 1: 14b5618945 difftool: create a tmpdir path without repeated slashes
@@ Commit message
Strip trailing slashes from the value read from TMPDIR to avoid
repeated slashes in the generated paths.
- Adjust the error handling to avoid leaking strbufs.
+ Adjust the error handling to avoid leaking strbufs and to avoid
+ returning -1 to cmd_main().
Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, co
strbuf_release(&buf);
+ strbuf_release(&tmpdir);
- return ret;
+- return ret;
++ return (ret < 0) ? 1 : ret;
}
+
+ static int run_file_diff(int prompt, const char *prefix,
## t/t7800-difftool.sh ##
@@ t/t7800-difftool.sh: run_dir_diff_test 'difftool --dir-diff' '
4: 8e7d54616f ! 2: 0824321eb9 difftool: refactor dir-diff to write files using a helper function
@@ Metadata
Author: David Aguilar <davvid@xxxxxxxxx>
## Commit message ##
- difftool: refactor dir-diff to write files using a helper function
+ difftool: refactor dir-diff to write files using helper functions
- Add a write_entry() helper function to handle the unlinking and writing
+ Add a helpers function to handle the unlinking and writing
of the dir-diff submodule and symlink stand-in files.
- Use write_entry() inside of the hashmap loops to eliminate duplicate
- code and to safeguard the submodules hashmap loop against the
- symlink-chasing behavior that 5bafb3576a (difftool: fix symlink-file
- writing in dir-diff mode, 2021-09-22) addressed.
+ Use the helpers to implement the guts of the hashmap loops.
+ This eliminate duplicate code and safeguards the submodules
+ hashmap loop against the symlink-chasing behavior that 5bafb3576a
+ (difftool: fix symlink-file writing in dir-diff mode, 2021-09-22)
+ addressed.
The submodules loop should not strictly require the unlink() call that
this is introducing to them, but it does not necessarily hurt them
@@ builtin/difftool.c: static int checkout_path(unsigned mode, struct object_id *oi
return ret;
}
-+static void write_entry(const char *path, const char *content,
-+ struct strbuf *buf, size_t len)
++static void write_file_in_directory(struct strbuf *dir, size_t dir_len,
++ const char *path, const char *content)
+{
-+ if (!*content)
-+ return;
-+ add_path(buf, len, path);
-+ ensure_leading_directories(buf->buf);
-+ unlink(buf->buf);
-+ write_file(buf->buf, "%s", content);
++ add_path(dir, dir_len, path);
++ ensure_leading_directories(dir->buf);
++ unlink(dir->buf);
++ write_file(dir->buf, "%s", content);
++}
++
++/* Write the file contents for the left and right sides of the difftool
++ * dir-diff representation for submodules and symlinks. Symlinks and submodules
++ * are written as regular text files so that external diff tools can diff them
++ * as text files, resulting in behavior that is analogous to to what "git diff"
++ * displays for symlink and submodule diffs.
++ */
++static void write_standin_files(struct pair_entry *entry,
++ struct strbuf *ldir, size_t ldir_len,
++ struct strbuf *rdir, size_t rdir_len)
++{
++ if (*entry->left)
++ write_file_in_directory(ldir, ldir_len, entry->path, entry->left);
++ if (*entry->right)
++ write_file_in_directory(rdir, rdir_len, entry->path, entry->right);
+}
+
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, co
- ensure_leading_directories(rdir.buf);
- write_file(rdir.buf, "%s", entry->right);
- }
-+ write_entry(entry->path, entry->left, &ldir, ldir_len);
-+ write_entry(entry->path, entry->right, &rdir, rdir_len);
++ write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
}
/*
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, co
- write_file(rdir.buf, "%s", entry->right);
- }
+
-+ write_entry(entry->path, entry->left, &ldir, ldir_len);
-+ write_entry(entry->path, entry->right, &rdir, rdir_len);
++ write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
}
strbuf_release(&buf);
5: 8db6ae3373 ! 3: 94ad86157e difftool: remove an unnecessary call to strbuf_release()
@@ Commit message
## builtin/difftool.c ##
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
- write_entry(entry->path, entry->right, &rdir, rdir_len);
+ write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
}
- strbuf_release(&buf);
2: 080a113917 = 4: 5b6dfe5e5c difftool: add a missing space to the run_dir_diff() comments
3: 1fbc47a58d < -: ---------- difftool: avoid returning -1 to cmd_main() from run_dir_diff()
--
2.33.0.886.g5b6dfe5e5c
