From: Paolo Giarrusso Date: Wed, 09 Oct 2013 10:32:31 GMT Subject: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly Message-ID: In-Reply-To: (Resending without HTML). On Wed, Oct 9, 2013 at 12:20 PM, Tay Ray Chuan wrote: > > On Wed, Oct 9, 2013 at 11:57 AM, Paolo G. Giarrusso > wrote: > > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > > index 7d7af03..ebfb78f 100755 > > --- a/contrib/subtree/git-subtree.sh > > +++ b/contrib/subtree/git-subtree.sh > > @@ -592,7 +592,9 @@ cmd_split() > > eval "$grl" | > > while read rev parents; do > > revcount=$(($revcount + 1)) > > - say -n "$revcount/$revmax ($createcount) > > " > > + if [ -z "$quiet" ]; then > > + printf "%s" "$revcount/$revmax ($createcount) > > " >&2 An additional note for reviewers and appliers: the original and the patched codeboth embed a literal ^M, not a new line, go to back to the beginning of the line and overwrite it, so the above is not a consequence of line-wrap. I used git-format-patch and git-send-email, and the ^M is visible in Vim in the exported patch (that's why I didn't remark on it). Seeing the email, I wonder whether there's hope something like that can be preserved in an email, and whether the code should use some escape sequence instead. > > + fi > > Reviewers might wish to know that "say" in git-subtree is defined as > > say() > { > if [ -z "$quiet" ]; then > echo "$@" >&2 > fi > } > > Hence the "if" and the redirect. Indeed. I considered having a variant of `say` instead of inlining and customizing it, but for once I decided to keep this simple, since this variant of `say` is currently used only once. Otherwise, one could change say to use printf, but that's more invasive. Cheers, -- Paolo G. Giarrusso - Ph.D. Student, Philipps-University Marburg http://www.informatik.uni-marburg.de/~pgiarrusso/