From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 11 Feb 2010 23:21:32 GMT
Subject: Re: [PATCH 3/4] git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit.
Message-ID: <7vocjvqryb.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1265923579-24900-4-git-send-email-stefan.hahn@s-hahn.de>

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:

> If applying patches with the following command sequence
>
>    git format-patch --stdout ... | git am ...
>
> in repositories having files with dos and unix line endings
> git-mailsplit, which is called from git-am must be called with
> `--keep-cr` parameter since commit c2ca1d79.

Sorry, I cannot parse this.

Perhaps you meant to have a comma between "git-am" and "must be"?

> +e,keep-cr       pass --keep-cr flag to git-mailsplit for mbox format

This short form -e does not make much sense to me.  Why is it -e, and do
we even need a short form in the first place?

I'd say we should drop it.  We may want to use short-and-sweet 'e' for
something more important and common, and we would regret for letting this
option squat on it later.

Another approach _might_ be to let the user to use --rebasing directly; it
currently is documented as "internal use", but as long as we clearly
specify its semantics and give a synonym that is more sensible than the
current name, it might turn out to be a better option.  I dunno; it might
be doing more than what this new use case may want to do.

> @@ -216,10 +217,12 @@ check_patch_format () {
>  split_patches () {
>  	case "$patch_format" in
>  	mbox)
> -		case "$rebasing" in
> -		'')
> +		case "$rebasing,$keepcr" in
> +		'','')
>  			keep_cr= ;;
> -		?*)
> +		'',t)
> +			keep_cr=--keep-cr ;;
> +		?*,t)
>  			keep_cr=--keep-cr ;;

Did you mean to say:

	case "$r$k" in
        '') keep_cr= ;;
        ?*) keep_cr=--keep-cr ;;
        esac

or even:

	if test -n "$r$k"
        then
        	keep_cr=--keep-cr
	else
        	keep_cr=
	fi