Junio C Hamano <gitster@xxxxxxxxx> writes:
> After re-reading the code that parses the command line options given to
> "am" and the previous invocation state we read from $dotest/*, however, I
> think the way this change uses $keep makes things somewhat inconsistent
> and harder to follow.
>
> Currently the variables are given abstract meaning (e.g. "are we told to
> record to utf8? yes or no") when we parse our command line options and
> read from the previous invocation state, and then based on that abstract
> meaning, a later code decides what exact option we throw at the git
> commands we invoke (e.g. "utf8=t" -> "-u").
>
> How about doing something like this instead at least for now? It might be
> better to decide when we parse our options and $dotest/* immediately what
> options we give to the git commands we run (which your patch does but only
> to $keep option), but that kind of change (1) belongs to a separate topic
> and should be done consistently to all options, and (2) I am not convinced
> if it is necessarily a good change.
Yes, at second glance it's probably better to remain consistent. I
didn't like it at first because it's layering complexity on it, but you
are right, the existing code follows the same pattern.
> diff --git a/git-am.sh b/git-am.sh
> index 6cdd591..8b755d9 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -15,6 +15,7 @@ q,quiet be quiet
> s,signoff add a Signed-off-by line to the commit message
> u,utf8 recode into utf8 (default)
> k,keep pass -k flag to git-mailinfo
> +keep-non-patch pass -b flag to git-mailinfo
> keep-cr pass --keep-cr flag to git-mailsplit for mbox format
> no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
> c,scissors strip everything before a scissors line
> @@ -345,6 +346,8 @@ do
> utf8= ;;
> -k|--keep)
> keep=t ;;
> + --keep-non-patch)
> + keep=b ;;
> -c|--scissors)
> scissors=t ;;
> --no-scissors)
> @@ -522,16 +525,25 @@ case "$resolved" in
> fi
> esac
>
> +# Now, decide what command line options we will give to the git
> +# commands we invoke, based on the result of parsing command line
> +# options and previous invocation state stored in $dotest/ files.
> +
> if test "$(cat "$dotest/utf8")" = t
> then
> utf8=-u
> else
> utf8=-n
> fi
> -if test "$(cat "$dotest/keep")" = t
> -then
> - keep=-k
> -fi
> +keep=$(cat "$dotest/keep")
> +case "$keep" in
> +t)
> + keep=-k ;;
> +b)
> + keep=-b ;;
> +*)
> + keep= ;;
> +esac
> case "$(cat "$dotest/keepcr")" in
> t)
> keepcr=--keep-cr ;;
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html