Re: [PATCH 11/11] i18n: difftool: mark warnings for translation

[David Aguilar <davvid@gmail.com>]

On Tue, Jun 21, 2016 at 11:44:13AM +0000, Vasco Almeida wrote:
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -451,11 +452,11 @@ sub dir_diff
>  		}
>  
>  		if (exists $wt_modified{$file} and exists $tmp_modified{$file}) {
> -			my $errmsg = "warning: Both files modified: ";
> -			$errmsg .= "'$workdir/$file' and '$b/$file'.\n";
> -			$errmsg .= "warning: Working tree file has been left.\n";
> -			$errmsg .= "warning:\n";
> -			warn $errmsg;
> +			warn sprintf __(
> +"warning: Both files modified:
> +'%s/%s' and '%s/%s'.
> +warning: Working tree file has been left.
> +warning:\n"), $workdir, $file, $b, $file;


Sorry for the nit, but this seems a little hard to read.

It's unfortunate that we have to lose the flow of the code here.
Perhaps we can do something like this to make it flow more like
the original instead?

+			warn sprintf(__(
+				"warning: Both files modified:\n" .
+				"'%s/%s' and '%s/%s'.\n" .
+				"warning: Working tree file has been left.\n" .
+				"warning:\n"), $workdir, $file, $b, $file);

I tend to also prefer parens on the sprintf so that the
parameter grouping is easier to see.

> @@ -467,8 +468,9 @@ sub dir_diff
>  		}
>  	}
>  	if ($error) {
> -		warn "warning: Temporary files exist in '$tmpdir'.\n";
> -		warn "warning: You may want to cleanup or recover these.\n";
> +		warn sprintf __(
> +"warning: Temporary files exist in '%s'.
> +warning: You may want to cleanup or recover these.\n"), $tmpdir;

Ditto, perhaps something like...

+		warn sprintf(__(
+			"warning: Temporary files exist in '%s'.\n" .
+			"warning: You may want to cleanup or recover these.\n"
+			), $tmpdir);

I'm assuming that the i18n infrastructure is prepared to deal
with perl's . dot syntax for string concatenation, though, and
I don't know whether that's true.  Does that work?

What do you think?

ciao,
-- 
David