Re: [TopGit PATCH] tg-create.sh: Support for multiple {to,cc,bcc} options

Petr Baudis [off-list ref] writes:

quoted

+	prefix="$(echo "$2" | sed -e 's/\//\\\\\//g')"

Maybe use s### ? ;-)

Personally I like '|' instead.  It's much less visually distracting than #.

quoted

+
+	git config --get-all topgit.$1 2>/dev/null |
+		sed -e "s/^/$prefix /g"
+}

Won't this return an error code and terminate the script in case no
option is defined?

Exit code from upstream of a pipe does not affect the exit code from the
pipeline, and sed does not exit non-zero just because there was no
substitution.

quoted

-	! header="$(git config topgit.to)" || echo "To: $header"
-	! header="$(git config topgit.cc)" || echo "Cc: $header"
-	! header="$(git config topgit.bcc)" || echo "Bcc: $header"
+	get_multi_config to  "To:"
+	get_multi_config cc  "Cc:"
+	get_multi_config bcc "Bcc:"
 	! subject_prefix="$(git config topgit.subjectprefix)" || subject_prefix="$subject_prefix "
 	echo "Subject: [${subject_prefix}PATCH] $name"
 	echo

One trouble here is that I've seen mailers mess up when there is
multiple occurences of these headers, so it would be probably safer to
concatenate them all to single line, comma-separated.

It is not just "I've seen mailers"; RFC2822 wants you to have at most one
(see the table on Page 20).

On Sat, Aug 9, 2008 at 03:37, Junio C Hamano [off-list ref] wrote:

Petr Baudis [off-list ref] writes:

quoted

quoted

+    prefix="$(echo "$2" | sed -e 's/\//\\\\\//g')"

Maybe use s### ? ;-)

Personally I like '|' instead.  It's much less visually distracting than #.

  ^^^^^^^^^^ says it all ;-)

quoted

quoted

+
+    git config --get-all topgit.$1 2>/dev/null |
+            sed -e "s/^/$prefix /g"
+}

Won't this return an error code and terminate the script in case no
option is defined?

I tested it with none defined options: no error, no distracting empty
lines, no error messages. so it worked here.

quoted

quoted

-    ! header="$(git config topgit.to)" || echo "To: $header"
-    ! header="$(git config topgit.cc)" || echo "Cc: $header"
-    ! header="$(git config topgit.bcc)" || echo "Bcc: $header"
+    get_multi_config to  "To:"
+    get_multi_config cc  "Cc:"
+    get_multi_config bcc "Bcc:"
     ! subject_prefix="$(git config topgit.subjectprefix)" || subject_prefix="$subject_prefix "
     echo "Subject: [${subject_prefix}PATCH] $name"
     echo

One trouble here is that I've seen mailers mess up when there is
multiple occurences of these headers, so it would be probably safer to
concatenate them all to single line, comma-separated.

It is not just "I've seen mailers"; RFC2822 wants you to have at most one
(see the table on Page 20).

But do we generate a valid mail with tg patch, or just a patch file
with some special looking lines? Anyway, I thought about the comma
separated solution too, but git send-mail handles these multi lines
well. So I take the easy road.

Bert

On Fri, Aug 08, 2008 at 06:37:13PM -0700, Junio C Hamano wrote:

Petr Baudis [off-list ref] writes:

quoted

quoted

+	prefix="$(echo "$2" | sed -e 's/\//\\\\\//g')"

Maybe use s### ? ;-)

Personally I like '|' instead.  It's much less visually distracting than #.

I'm just used to '#' more, but either is fine for me.

quoted

quoted

+
+	git config --get-all topgit.$1 2>/dev/null |
+		sed -e "s/^/$prefix /g"
+}

Won't this return an error code and terminate the script in case no
option is defined?

Exit code from upstream of a pipe does not affect the exit code from the
pipeline, and sed does not exit non-zero just because there was no
substitution.

Oh, you're right - somehow I didn't quite absorb that pipe is used late
at the night. ;-) I still don't understand the stderr redirect, though.

				Petr "Pasky" Baudis

On Sat, Aug 9, 2008 at 10:27, Petr Baudis [off-list ref] wrote:

quoted

quoted

quoted

+
+  git config --get-all topgit.$1 2>/dev/null |
+          sed -e "s/^/$prefix /g"
+}

I still don't understand the stderr redirect, though.

Just precaution, I think. It seems unnecessary.

Bert

                               Petr "Pasky" Baudis