git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v7 2/4] difftool: refactor dir-diff to write files using helper functions

From
David Aguilar <davvid@gmail.com>
Date
Oct 1, 2021, 01:37 UTC
Message-ID
<20211001013756.37586-3-davvid@gmail.com>
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
Previous: David AguilarNext: David Aguilar
Message 3 of 5 in “difftool: dir-diff improvements and refactoring”
  1. 0/4 difftool: dir-diff improvements and refactoringDavid Aguilar, Oct 1, 2021
  2. 1/4 difftool: create a tmpdir path without repeated slashesDavid Aguilar, Oct 1, 2021
  3. 2/4 difftool: refactor dir-diff to write files using helper functionsDavid Aguilar, Oct 1, 2021
  4. 3/4 difftool: remove an unnecessary call to strbuf_release()David Aguilar, Oct 1, 2021
  5. 4/4 difftool: add a missing space to the run_dir_diff() commentsDavid Aguilar, Oct 1, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.