git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/2] am: learn passing -b to mailinfo

From
Thomas Rast <trast@student.ethz.ch>
Date
Jan 20, 2012, 13:04 UTC
Message-ID
<87ehuucypo.fsf@thomas.inf.ethz.ch>
In-Reply-To
<7vd3afidt3.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> 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.

Show 51 quoted lines
> 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
Previous: Junio C HamanoNext: Thomas Rast
Message 8 of 9 in “mailinfo documentation: accurately describe non -k case”
  1. 1/3 mailinfo documentation: accurately describe non -k caseThomas Rast, Jan 11, 2012
  2. 2/3 am: learn passing -b to mailinfoThomas Rast, Jan 11, 2012
  3. Junio C HamanoJan 12, 2012
  4. Thomas RastJan 12, 2012
  5. 1/2 am: learn passing -b to mailinfoThomas Rast, Jan 16, 2012
  6. 2/2 mailinfo: with -b, keep space after [foo]Thomas Rast, Jan 16, 2012
  7. Junio C HamanoJan 19, 2012
  8. Thomas RastJan 20, 2012
  9. 3/3 mailinfo: with -b, keep space after [foo]Thomas Rast, Jan 11, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.