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 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

David Aguilar committed Sep 30, 2021 at 18:37 UTC 2255c80c91da9ed7fa127dad3aa92eb9f448c88b
1 file changed +28 -22
builtin/difftool.c
+28 -22
@@ -320,6 +320,31 @@ static int checkout_path(unsigned mode, struct object_id *oid,
320 return ret;
321 }
322
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 +
348 static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
349 struct child_process *child)
350 {
@@ -529,16 +554,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
554 */
555 hashmap_for_each_entry(&submodules, &iter, entry,
556 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);
558 }
559
560 /*
@@ -548,18 +564,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
564 */
565 hashmap_for_each_entry(&symlinks2, &iter, entry,
566 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);
569 }
570
571 strbuf_release(&buf);