threads / patch / 31305

patchdifftool: silence warning

Subject: [PATCH] difftool: silence warning

## tl;dr

3 messages between Aug 21, 2012 and Aug 21, 2012. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ross Lagerwall· Aug 21, 2012, 06:59 UTC · lore

Silence a warning given when running git difftool --dir-diff and there are no changes. This is because command_oneline returns undef when the command has no output, not ''.

Signed-off-by: Ross Lagerwall <rosslagerwall@gmail.com>
---
 git-difftool.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to git-difftool.perl +1 −1

diff --git a/git-difftool.perl b/git-difftool.perl
index ae1e052..1cfcbb3 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -117,7 +117,7 @@ sub setup_dir_diff
 	# by Git->repository->command*.
 	my $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir);
 	my $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV);
-	exit(0) if (length($diffrtn) == 0);
+	exit(0) unless defined($diffrtn);
 
 	# Setup temp directories
 	my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);

-- 
1.7.11.4

David Aguilar· Aug 21, 2012, 09:04 UTC · re: Ross Lagerwall · lore

Re: [PATCH] difftool: silence warning

On Mon, Aug 20, 2012 at 11:59 PM, Ross Lagerwall <rosslagerwall@gmail.com> wrote:

Show 6 quoted lines

> Silence a warning given when running git difftool --dir-diff and
> there are no changes.
> This is because command_oneline returns undef when the command has no
> output, not ''.
>
> Signed-off-by: Ross Lagerwall <rosslagerwall@gmail.com>

Thanks

This patch is obviously correct, but it won't apply in git's "next" branch. Can you please prepare a patch based on the version in next?

A small question on Perl style for the list... is it better say this?

   exit(0) unless $diffrtn;

or is it better to explicitly check for undef using defined($diffrtn) like was done in this patch? I would assume that explicit is preferred.

Show 19 quoted lines

> ---
>  git-difftool.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-difftool.perl b/git-difftool.perl
> index ae1e052..1cfcbb3 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -117,7 +117,7 @@ sub setup_dir_diff
>         # by Git->repository->command*.
>         my $diffrepo = Git->repository(Repository => $repo_path, WorkingCopy => $workdir);
>         my $diffrtn = $diffrepo->command_oneline('diff', '--raw', '--no-abbrev', '-z', @ARGV);
> -       exit(0) if (length($diffrtn) == 0);
> +       exit(0) unless defined($diffrtn);
>
>         # Setup temp directories
>         my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);
> --
> 1.7.11.4

-- 
David

Ross Lagerwall· Aug 21, 2012, 09:42 UTC · re: David Aguilar · lore

Re: [PATCH] difftool: silence warning

On 08/21/2012 11:04 AM, David Aguilar wrote:

> This patch is obviously correct, but it won't apply in git's "next" branch.
> Can you please prepare a patch based on the version in next?
>

Sure.

Show 8 quoted lines

> 
> A small question on Perl style for the list... is it better say this?
> 
>    exit(0) unless $diffrtn;
> 
> or is it better to explicitly check for undef using defined($diffrtn)
> like was done in this patch? I would assume that explicit is preferred.
>

It seems like defined() is used throughout git-difftool so I just used that.

Regards

-- 
Ross Lagerwall