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
Junio C Hamano <gitster@pobox.com>
Date
Jan 19, 2012, 21:26 UTC
Message-ID
<7vd3afidt3.fsf@alter.siamese.dyndns.org>
In-Reply-To
<a804650f805fd8c89a843302cb92bbbdf36b8c0b.1326710194.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 12 quoted lines
> git-am could pass -k to mailinfo, but not -b.  Introduce an option
> that does so.  We change the meaning of the 'keep' state file, but are
> careful not to cause a problem unless you downgrade in the middle of
> an 'am' run.
>
> This uncovers a bug in mailinfo -b, hence the failing test.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> This fixes the broken 'if', and the use of 'echo' with an argument
> that starts with '-'.

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.

Thanks.
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 ;;
Previous: Thomas RastNext: Thomas Rast
Message 7 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.