Skip to content

Commit 2255c80

Browse files
davvidgitster
authored andcommitted
difftool: refactor dir-diff to write files using helper functions
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 5bafb35 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4ac9f15 commit 2255c80

File tree

1 file changed

+28
-22
lines changed

1 file changed

+28
-22
lines changed

builtin/difftool.c

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,31 @@ static int checkout_path(unsigned mode, struct object_id *oid,
320320
return ret;
321321
}
322322

323+
static void write_file_in_directory(struct strbuf *dir, size_t dir_len,
324+
const char *path, const char *content)
325+
{
326+
add_path(dir, dir_len, path);
327+
ensure_leading_directories(dir->buf);
328+
unlink(dir->buf);
329+
write_file(dir->buf, "%s", content);
330+
}
331+
332+
/* Write the file contents for the left and right sides of the difftool
333+
* dir-diff representation for submodules and symlinks. Symlinks and submodules
334+
* are written as regular text files so that external diff tools can diff them
335+
* as text files, resulting in behavior that is analogous to to what "git diff"
336+
* displays for symlink and submodule diffs.
337+
*/
338+
static void write_standin_files(struct pair_entry *entry,
339+
struct strbuf *ldir, size_t ldir_len,
340+
struct strbuf *rdir, size_t rdir_len)
341+
{
342+
if (*entry->left)
343+
write_file_in_directory(ldir, ldir_len, entry->path, entry->left);
344+
if (*entry->right)
345+
write_file_in_directory(rdir, rdir_len, entry->path, entry->right);
346+
}
347+
323348
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
324349
struct child_process *child)
325350
{
@@ -529,16 +554,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
529554
*/
530555
hashmap_for_each_entry(&submodules, &iter, entry,
531556
entry /* member name */) {
532-
if (*entry->left) {
533-
add_path(&ldir, ldir_len, entry->path);
534-
ensure_leading_directories(ldir.buf);
535-
write_file(ldir.buf, "%s", entry->left);
536-
}
537-
if (*entry->right) {
538-
add_path(&rdir, rdir_len, entry->path);
539-
ensure_leading_directories(rdir.buf);
540-
write_file(rdir.buf, "%s", entry->right);
541-
}
557+
write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
542558
}
543559

544560
/*
@@ -548,18 +564,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
548564
*/
549565
hashmap_for_each_entry(&symlinks2, &iter, entry,
550566
entry /* member name */) {
551-
if (*entry->left) {
552-
add_path(&ldir, ldir_len, entry->path);
553-
ensure_leading_directories(ldir.buf);
554-
unlink(ldir.buf);
555-
write_file(ldir.buf, "%s", entry->left);
556-
}
557-
if (*entry->right) {
558-
add_path(&rdir, rdir_len, entry->path);
559-
ensure_leading_directories(rdir.buf);
560-
unlink(rdir.buf);
561-
write_file(rdir.buf, "%s", entry->right);
562-
}
567+
568+
write_standin_files(entry, &ldir, ldir_len, &rdir, rdir_len);
563569
}
564570

565571
strbuf_release(&buf);

0 commit comments

Comments
 (0)