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

From: David Aguilar <davvid@gmail.com>
Date: Jun 22, 2016, 04:33 UTC
Message-ID: <20160622043329.GA31643@gmail.com>
In-Reply-To: <20160621114413.9467-11-vascomalmeida@sapo.pt>

On Tue, Jun 21, 2016 at 11:44:13AM +0000, Vasco Almeida wrote:

Show 16 quoted lines

> --- 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.

Show 9 quoted lines

> @@ -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