Re: [PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

From: Theodore Tso <tytso@mit.edu>
Date: Mar 29, 2007, 17:09 UTC
Message-ID: <20070329170914.GA9435@thunk.org>
In-Reply-To: <20070329155741.GA18413@moooo.ath.cx>

On Thu, Mar 29, 2007 at 05:57:41PM +0200, Matthias Lederhofer wrote:

> The subject should probably refer to printf(1) instead of printf(3).

Yep.

> Theodore Ts'o <tytso@mit.edu> wrote:
> > +	printf  "'$path' was deleted"
> Here is one space too much after printf.

This was fixed in a later patch.

> > +	printf "'$path' is a symlink containing '"
> You should use printf "'%s' [..]" "$path" as you do in some other
> places (in case $path contains conversion specifiers).

This was also obsoleted in a later patch, but you're right.

> > +    printf "Hit return to start merge resolution tool ($merge_tool): "
> Here it is much more unlikely that $merge_tool contains a conversion
> specifier but anyway I'd prefer to use %s.

Yep, agreed.

Junio, I've prepared an alterate patch series. If you haven't pulled from repo.or.cz, I'd appreciate it if you could pull from the mergetool branch (instead of master) instead:

	http://repo.or.cz/w/git/mergetool.git

The differences between the patch series in the master and mergetool branch are:

	* Change the description of the printf patch to say: 
		printf(3)->printf(1)
	* Fix the double space after printf and symlink %s conversion
		earlier in the patch series (only to have it disappear
		in the last patch, so this is a no-op at the end of
		the day, but more of an excuse to go play with stgit
		and in the name of anal-rententive correctness)
	* Use a %s printf conversion for the "Hit return to start merge..."
		statement as noted above in the last patch.

Thanks!!

(Apparently there's no way to forcibly reset the pointer of the master head on repo.or.cz, probably for security/sanity reasons, without going through an administrator.)

						- Ted