From: David Aguilar <davvid@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: [PATCH v7 2/4] difftool: refactor dir-diff to write files using helper functions
Date: Thu, 30 Sep 2021 18:37:54 -0700 [thread overview]
Message-ID: <20211001013756.37586-3-davvid@gmail.com> (raw)
In-Reply-To: <20211001013756.37586-1-davvid@gmail.com>
Add a helpers function to handle the unlinking and writing
of the dir-diff submodule and symlink stand-in files.
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
either beyond the cost of the extra unlink().
Signed-off-by: David Aguilar <davvid@gmail.com>
---
builtin/difftool.c | 50 ++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 0e24421682..f3cd1e5b53 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -320,6 +320,31 @@ static int checkout_path(unsigned mode, struct object_id *oid,
return ret;
}
+static void write_file_in_directory(struct strbuf *dir, size_t dir_len,
+ const char *path, const char *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,
struct child_process *child)
{
@@ -529,16 +554,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
*/
hashmap_for_each_entry(&submodules, &iter, entry,
entry /* member name */) {
- if (*entry->left) {
- add_path(&ldir, ldir_len, entry->path);
- ensure_leading_directories(ldir.buf);
- write_file(ldir.buf, "%s", entry->left);
- }
- if (*entry->right) {
- add_path(&rdir, rdir_len, entry->path);
- ensure_leading_directories(rdir.buf);
- write_file(rdir.buf, "%s", entry->right);
- }
+ write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
}
/*
@@ -548,18 +564,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
*/
hashmap_for_each_entry(&symlinks2, &iter, entry,
entry /* member name */) {
- if (*entry->left) {
- add_path(&ldir, ldir_len, entry->path);
- ensure_leading_directories(ldir.buf);
- unlink(ldir.buf);
- write_file(ldir.buf, "%s", entry->left);
- }
- if (*entry->right) {
- add_path(&rdir, rdir_len, entry->path);
- ensure_leading_directories(rdir.buf);
- unlink(rdir.buf);
- write_file(rdir.buf, "%s", entry->right);
- }
+
+ write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
}
strbuf_release(&buf);
--
2.33.0.886.g5b6dfe5e5c
next prev parent reply other threads:[~2021-10-01 1:38 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 1:37 [PATCH v7 0/4] difftool: dir-diff improvements and refactoring David Aguilar
2021-10-01 1:37 ` [PATCH v7 1/4] difftool: create a tmpdir path without repeated slashes David Aguilar
2021-10-01 1:37 ` David Aguilar [this message]
2021-10-01 1:37 ` [PATCH v7 3/4] difftool: remove an unnecessary call to strbuf_release() David Aguilar
2021-10-01 1:37 ` [PATCH v7 4/4] difftool: add a missing space to the run_dir_diff() comments David Aguilar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211001013756.37586-3-davvid@gmail.com \
--to=davvid@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).