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

[PATCH 3/3] difftool: use Git::* functions instead of passing around state

From
David Aguilar <davvid@gmail.com>
Date
Jul 19, 2016, 03:57 UTC
Message-ID
<20160719035756.24961-3-davvid@gmail.com>
In-Reply-To
<20160719035756.24961-1-davvid@gmail.com>

Call Git::command() and friends directly wherever possible. This makes it clear that these operations can be invoked directly without needing to manage the current directory and related GIT_* environment variables.

Eliminate find_repository() since we can now use wc_path() and not worry about side-effects involving environment variables.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl | 54 ++++++++++++++++++++++--------------------------------
 1 file changed, 22 insertions(+), 32 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index bc2267f..a5790d0 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -37,14 +37,6 @@ USAGE
 	exit($exitcode);
 }
 
-sub find_worktree
-{
-	# Git->repository->wc_path() does not honor changes to the working
-	# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'
-	# config variable.
-	return Git::command_oneline('rev-parse', '--show-toplevel');
-}
-
 sub print_tool_help
 {
 	# See the comment at the bottom of file_diff() for the reason behind
@@ -67,14 +59,14 @@ sub exit_cleanup
 
 sub use_wt_file
 {
-	my ($repo, $workdir, $file, $sha1) = @_;
+	my ($workdir, $file, $sha1) = @_;
 	my $null_sha1 = '0' x 40;
 
 	if (-l "$workdir/$file" || ! -e _) {
 		return (0, $null_sha1);
 	}
 
-	my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+	my $wt_sha1 = Git::command_oneline('hash-object', "$workdir/$file");
 	my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1);
 	return ($use, $wt_sha1);
 }
@@ -88,11 +80,11 @@ sub changed_files
 	my @refreshargs = (
 		@gitargs, 'update-index',
 		'--really-refresh', '-q', '--unmerged');
-	my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
 	try {
 		Git::command_oneline(@refreshargs);
 	} catch Git::Error::Command with {};
 
+	my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
 	my $line = Git::command_oneline(@diffargs);
 	my @files;
 	if (defined $line) {
@@ -108,11 +100,9 @@ sub changed_files
 
 sub setup_dir_diff
 {
-	my ($repo, $workdir, $symlinks) = @_;
-
-	my $repo_path = $repo->repo_path();
+	my ($workdir, $symlinks) = @_;
 	my @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV);
-	my $diffrtn = $repo->command_oneline(@gitargs);
+	my $diffrtn = Git::command_oneline(@gitargs);
 	exit(0) unless defined($diffrtn);
 
 	# Build index info for left and right sides of the diff
@@ -164,12 +154,12 @@ EOF
 
 		if ($lmode eq $symlink_mode) {
 			$symlink{$src_path}{left} =
-				$repo->command_oneline('show', "$lsha1");
+				Git::command_oneline('show', $lsha1);
 		}
 
 		if ($rmode eq $symlink_mode) {
 			$symlink{$dst_path}{right} =
-				$repo->command_oneline('show', "$rsha1");
+				Git::command_oneline('show', $rsha1);
 		}
 
 		if ($lmode ne $null_mode and $status !~ /^C/) {
@@ -181,8 +171,8 @@ EOF
 			if ($working_tree_dups{$dst_path}++) {
 				next;
 			}
-			my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
-							  $dst_path, $rsha1);
+			my ($use, $wt_sha1) =
+				use_wt_file($workdir, $dst_path, $rsha1);
 			if ($use) {
 				push @working_tree, $dst_path;
 				$wtindex .= "$rmode $wt_sha1\t$dst_path\0";
@@ -203,27 +193,27 @@ EOF
 	my ($inpipe, $ctx);
 	$ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
 	($inpipe, $ctx) =
-		$repo->command_input_pipe(qw(update-index -z --index-info));
+		Git::command_input_pipe('update-index', '-z', '--index-info');
 	print($inpipe $lindex);
-	$repo->command_close_pipe($inpipe, $ctx);
+	Git::command_close_pipe($inpipe, $ctx);
 
 	my $rc = system('git', 'checkout-index', '--all', "--prefix=$ldir/");
 	exit_cleanup($tmpdir, $rc) if $rc != 0;
 
 	$ENV{GIT_INDEX_FILE} = "$tmpdir/rindex";
 	($inpipe, $ctx) =
-		$repo->command_input_pipe(qw(update-index -z --index-info));
+		Git::command_input_pipe('update-index', '-z', '--index-info');
 	print($inpipe $rindex);
-	$repo->command_close_pipe($inpipe, $ctx);
+	Git::command_close_pipe($inpipe, $ctx);
 
 	$rc = system('git', 'checkout-index', '--all', "--prefix=$rdir/");
 	exit_cleanup($tmpdir, $rc) if $rc != 0;
 
 	$ENV{GIT_INDEX_FILE} = "$tmpdir/wtindex";
 	($inpipe, $ctx) =
-		$repo->command_input_pipe(qw(update-index --info-only -z --index-info));
+		Git::command_input_pipe('update-index', '--info-only', '-z', '--index-info');
 	print($inpipe $wtindex);
-	$repo->command_close_pipe($inpipe, $ctx);
+	Git::command_close_pipe($inpipe, $ctx);
 
 	# If $GIT_DIR was explicitly set just for the update/checkout
 	# commands, then it should be unset before continuing.
@@ -393,9 +383,9 @@ sub dir_diff
 	my $rc;
 	my $error = 0;
 	my $repo = Git->repository();
-	my $workdir = find_worktree();
-	my ($a, $b, $tmpdir, @worktree) =
-		setup_dir_diff($repo, $workdir, $symlinks);
+	my $repo_path = $repo->repo_path();
+	my $workdir = $repo->wc_path();
+	my ($a, $b, $tmpdir, @worktree) = setup_dir_diff($workdir, $symlinks);
 
 	if (defined($extcmd)) {
 		$rc = system($extcmd, $a, $b);
@@ -421,10 +411,10 @@ sub dir_diff
 		next if ! -f "$b/$file";
 
 		if (!$indices_loaded) {
-			%wt_modified = changed_files($repo->repo_path(),
-				"$tmpdir/wtindex", "$workdir");
-			%tmp_modified = changed_files($repo->repo_path(),
-				"$tmpdir/wtindex", "$b");
+			%wt_modified = changed_files(
+				$repo_path, "$tmpdir/wtindex", $workdir);
+			%tmp_modified = changed_files(
+				$repo_path, "$tmpdir/wtindex", $b);
 			$indices_loaded = 1;
 		}
 
-- 
2.9.2.280.g385e27a
Previous: David AguilarNext: Junio C Hamano
Message 6 of 8 in “difftool: fix argument handling in subdirs”
  1. 1/3 difftool: fix argument handling in subdirsDavid Aguilar, Jul 19, 2016
  2. 2/3 difftool: avoid $GIT_DIR and $GIT_WORK_TREEDavid Aguilar, Jul 19, 2016
  3. Junio C HamanoJul 19, 2016
  4. Junio C HamanoJul 19, 2016
  5. David AguilarJul 20, 2016
  6. 3/3 difftool: use Git::* functions instead of passing around stateDavid Aguilar, Jul 19, 2016
  7. Junio C HamanoJul 19, 2016
  8. Junio C HamanoJul 19, 2016

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.