threads / patch / 22619

patch, 4 partsgit-mailsplit: Show parameter '--keep-cr' in usage and documentation

Subject: [PATCH 1/4] git-mailsplit: Show parameter '--keep-cr' in usage and documentation

## tl;dr

13 messages between Feb 11, 2010 and Feb 12, 2010. Diffs are folded; open one to read it.

replies: 12people: 2as markdown or json

Stefan-W. Hahn· Feb 11, 2010, 21:26 UTC · lore

[PATCHv2 0/4] Using git-mailsplit in mixed line ending environment

Hello,

I'm using git in environments with files having dos or unix line ending. I apply patches using 'git format-patch ... | git am ...'. A change in git-mailsplit in commit c2ca1d79 introduced a change in the default behaviour of git-mailsplit when splitting mbox patches. It makes dos line endings to unix line endings. With this behaviour it is impossible to apply patches.

The following patches introduce the '--kepp-cr' parameter to git-am an an additional possibility to set '--keep-cr' via configuration for git-mailsplit. Also I added missing description for '--keep-cr' of git-mailsplit.

Second round: I changed 'mailsplit.keep-cr' to 'mailsplit.keepcr' as suggested by Jakub and a comment in the testcase.

Stefan-W. Hahn (4):
  git-mailsplit: Show parameter '--keep-cr' in usage and documentation
  git-mailsplit: add `mailsplit.keepcr` configuration variable.
  git-am: Add command line parameter `--keep-cr` passing it to
    git-mailsplit.
  Adding test for `--keep-cr` for git-mailsplit and git-am.
 Documentation/config.txt        |    4 ++
 Documentation/git-mailsplit.txt |    5 ++-
 builtin-mailsplit.c             |   12 ++++++-
 git-am.sh                       |   18 +++++++--
 t/t5101-mailinfo-dos.sh         |   74 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 107 insertions(+), 6 deletions(-)
 create mode 100644 t/t5101-mailinfo-dos.sh
Stefan-W. Hahn· Feb 11, 2010, 21:26 UTC · re: Stefan-W. Hahn · lore
Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
---
 Documentation/git-mailsplit.txt |    5 ++++-
 builtin-mailsplit.c             |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)
Show changes to 2 files +5 −2

Documentation/git-mailsplit.txt, builtin-mailsplit.c

diff --git a/Documentation/git-mailsplit.txt b/Documentation/git-mailsplit.txt
index 5cc94ec..a634485 100644
--- a/Documentation/git-mailsplit.txt
+++ b/Documentation/git-mailsplit.txt
@@ -7,7 +7,7 @@ git-mailsplit - Simple UNIX mbox splitter program
 
 SYNOPSIS
 --------
-'git mailsplit' [-b] [-f<nn>] [-d<prec>] -o<directory> [--] [<mbox>|<Maildir>...]
+'git mailsplit' [-b] [-f<nn>] [-d<prec>] [--keep-cr] -o<directory> [--] [<mbox>|<Maildir>...]
 
 DESCRIPTION
 -----------
@@ -43,6 +43,9 @@ OPTIONS
 	Skip the first <nn> numbers, for example if -f3 is specified,
 	start the numbering with 0004.
 
+--keep-cr::
+	Do not remove `\r` from lines ending with `\r\n`.
+
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 207e358..cdfc1b7 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -10,7 +10,7 @@
 #include "strbuf.h"
 
 static const char git_mailsplit_usage[] =
-"git mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> [<mbox>|<Maildir>...]";
+"git mailsplit [-d<prec>] [-f<n>] [-b] [--keep-cr] -o<directory> [<mbox>|<Maildir>...]";
 
 static int is_from_line(const char *line, int len)
 {

-- 
1.7.0.rc2.17.g39385
Stefan-W. Hahn· Feb 11, 2010, 21:26 UTC · re: Stefan-W. Hahn · lore

[PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

If using git-mailsplit in environments where files with dos and unix line ending stay in one repository mbox patches must be split with additional parameter '--keep-cr', because the behaviour of git-mailsplit have been changed in commit c2ca1d79.

With this patch the behaviour of git-mailsplit can be set via configuration file.

Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
---
 Documentation/config.txt |    4 ++++
 builtin-mailsplit.c      |   10 ++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)
Show changes to 2 files +14 −0

Documentation/config.txt, builtin-mailsplit.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4c36aa9..37a4968 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1246,6 +1246,10 @@ mailmap.file::
 	subdirectory, or somewhere outside of the repository itself.
 	See linkgit:git-shortlog[1] and linkgit:git-blame[1].
 
+mailsplit.keepcr::
+	If true git-mailsplit will not remove `\r` from lines ending
+	with `\r\n`. See linkgit:git-mailsplit[1].
+
 man.viewer::
 	Specify the programs that may be used to display help in the
 	'man' format. See linkgit:git-help[1].
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index cdfc1b7..e76974d 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -210,6 +210,15 @@ out:
 	return ret;
 }
 
+static int git_mailsplit_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "mailsplit.keepcr")) {
+		keep_cr = git_config_bool(var, value);
+		return 0;
+	}
+	return git_default_config(var, value, cb);
+}
+
 int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 {
 	int nr = 0, nr_prec = 4, num = 0;
@@ -218,6 +227,7 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 	const char **argp;
 	static const char *stdin_only[] = { "-", NULL };
 
+	git_config(git_mailsplit_config, NULL);
 	for (argp = argv+1; *argp; argp++) {
 		const char *arg = *argp;
 

-- 
1.7.0.rc2.17.g39385
Junio C Hamano· Feb 11, 2010, 23:21 UTC · re: Stefan-W. Hahn · lore

Re: [PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
Show 7 quoted lines
> If using git-mailsplit in environments where files with dos and unix
> line ending stay in one repository mbox patches must be split with
> additional parameter '--keep-cr', because the behaviour of
> git-mailsplit have been changed in commit c2ca1d79.
>
> With this patch the behaviour of git-mailsplit can be set via
> configuration file.

This probably is a bad change. mailsplit is designed to be usable without being inside any git repository. With this change, doesn't the command start failing to run when run outside a git repository?

Stefan-W. Hahn· Feb 12, 2010, 21:03 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

Also sprach Junio C Hamano am Thu, 11 Feb 2010 at 15:21:52 -0800:
Show 6 quoted lines
> > With this patch the behaviour of git-mailsplit can be set via
> > configuration file.
> 
> This probably is a bad change.  mailsplit is designed to be usable without
> being inside any git repository.  With this change, doesn't the command
> start failing to run when run outside a git repository?
Ouch, I didn't mind this.

But perhaps it will be a better aproach to make such a configuration available for git-am. I will make a patch for this soon.

Stefan
-- 
Stefan-W. Hahn                          It is easy to make things.
/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.			

Please note that according to the German law on data retention,
information on every electronic information exchange with me is
retained for a period of six months.
Bundesgesetzblatt:
http://www.bgblportal.de/BGBL/bgbl1f/bgbl107s3198.pdf
Junio C Hamano· Feb 12, 2010, 21:49 UTC · re: Stefan-W. Hahn · lore

Re: [PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
> But perhaps it will be a better aproach to make such a configuration
> available for git-am. I will make a patch for this soon.

If a configuration is desired, I think that is probably a saner place to put it.

Regardless of where the configuration goes between am and mailsplit, you need to think about the interaction between rebase and am. You might set the configuration for am---should that automatically and always kick in when you run rebase? Do we need some way to override it per invocation when running rebase?

Junio C Hamano· Feb 12, 2010, 22:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> "Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
>
>> But perhaps it will be a better aproach to make such a configuration
>> available for git-am. I will make a patch for this soon.
>
> If a configuration is desired, I think that is probably a saner place to
> put it.
>
> Regardless of where the configuration goes between am and mailsplit, you
> need to think about the interaction between rebase and am.  You might set
> the configuration for am---should that automatically and always kick in
> when you run rebase?  Do we need some way to override it per invocation
> when running rebase?

Nah, I wasn't thinking straight. There is no reason to turn it off while rebase is running. That means under --rebasing mode, am should ignore the configuration and always do keep-cr, even when am.keepcr is set to false.

Sorry about the noise.
Stefan-W. Hahn· Feb 12, 2010, 22:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] git-mailsplit: add `mailsplit.keepcr` configuration variable.

Also sprach Junio C Hamano am Fri, 12 Feb 2010 at 13:49:12 -0800:
Show 5 quoted lines
> Regardless of where the configuration goes between am and mailsplit, you
> need to think about the interaction between rebase and am.  You might set
> the configuration for am---should that automatically and always kick in
> when you run rebase?  Do we need some way to override it per invocation
> when running rebase?

I did not have had any problem with git-rebase in mixed line ending environment. If used via git-rebase, git-mailsplit should called with --keep-cr set, independent from configuration for this.

Stefan
-- 
Stefan-W. Hahn                          It is easy to make things.
/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.			

Please note that according to the German law on data retention,
information on every electronic information exchange with me is
retained for a period of six months.
Bundesgesetzblatt:
http://www.bgblportal.de/BGBL/bgbl1f/bgbl107s3198.pdf
Stefan-W. Hahn· Feb 11, 2010, 21:26 UTC · re: Stefan-W. Hahn · lore

[PATCH 3/4] git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit.

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.

This patch adds the command line parameter `--keep-cr` for git-am.
Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
---
 git-am.sh |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)
Show changes to git-am.sh +14 −4
diff --git a/git-am.sh b/git-am.sh
index c8b9cbb..c3101ae 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
+e,keep-cr       pass --keep-cr flag to git-mailsplit for mbox format
 c,scissors      strip everything before a scissors line
 whitespace=     pass it through git-apply
 ignore-space-change pass it through git-apply
@@ -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 ;;
 		esac
 		git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > "$dotest/last" ||
@@ -290,7 +293,7 @@ split_patches () {
 
 prec=4
 dotest="$GIT_DIR/rebase-apply"
-sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
+sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
 resolvemsg= resume= scissors= no_inbody_headers=
 git_apply_opt=
 committer_date_is_author_date=
@@ -347,6 +350,8 @@ do
 		allow_rerere_autoupdate="$1" ;;
 	-q|--quiet)
 		GIT_QUIET=t ;;
+        -e|--keep-cr)
+                keepcr=t ;;
 	--)
 		shift; break ;;
 	*)
@@ -452,6 +457,7 @@ else
 	echo "$sign" >"$dotest/sign"
 	echo "$utf8" >"$dotest/utf8"
 	echo "$keep" >"$dotest/keep"
+	echo "$keepcr" >"$dotest/keepcr"
 	echo "$scissors" >"$dotest/scissors"
 	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
 	echo "$GIT_QUIET" >"$dotest/quiet"
@@ -495,6 +501,10 @@ if test "$(cat "$dotest/keep")" = t
 then
 	keep=-k
 fi
+if test "$(cat "$dotest/keepcr")" = t
+then
+	keepcr=--keep-cr
+fi
 case "$(cat "$dotest/scissors")" in
 t)
 	scissors=--scissors ;;

-- 
1.7.0.rc2.17.g39385
Junio C Hamano· Feb 11, 2010, 23:21 UTC · re: Stefan-W. Hahn · lore

Re: [PATCH 3/4] git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit.

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
Show 7 quoted lines
> 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.

Show 14 quoted lines
> @@ -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
Stefan-W. Hahn· Feb 12, 2010, 21:05 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/4] git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit.

Also sprach Junio C Hamano am Thu, 11 Feb 2010 at 15:21:32 -0800:
Show 11 quoted lines
> "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.
What?
Show 6 quoted lines
> 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?
Just for shortening the commandline.
Show 8 quoted lines
> or even:
> 
> 	if test -n "$r$k"
>         then
>         	keep_cr=--keep-cr
> 	else
>         	keep_cr=
> 	fi
This one. I will make a patch soon.
Stefan
-- 
Stefan-W. Hahn                          It is easy to make things.
/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.			

Please note that according to the German law on data retention,
information on every electronic information exchange with me is
retained for a period of six months.
Bundesgesetzblatt:
http://www.bgblportal.de/BGBL/bgbl1f/bgbl107s3198.pdf
Junio C Hamano· Feb 12, 2010, 21:54 UTC · re: Stefan-W. Hahn · lore

Re: [PATCH 3/4] git-am: Add command line parameter `--keep-cr` passing it to git-mailsplit.

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
Show 14 quoted lines
> Also sprach Junio C Hamano am Thu, 11 Feb 2010 at 15:21:32 -0800:
>> "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.
>
> What?
What I said.  I wasn't able to parse it.  If it were:
    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.

iow, two additional commas, I would have been able to, but I was not certain that is what you meant.

Show 6 quoted lines
>> > +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?
>
> Just for shortening the commandline.

That much I know already ;-) and you probably know that I knew when I wrote this, which you somehow omitted from your quote:

    ... 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.

The shorter one can come later after people find --keep-cr is something they often need to use and deserve a shorter form, but not until then.

Stefan-W. Hahn· Feb 11, 2010, 21:26 UTC · re: Stefan-W. Hahn · lore

[PATCH 4/4] Adding test for `--keep-cr` for git-mailsplit and git-am.

This test adds test for git-mailsplit using dos line endings, the command sequence 'git format-patch ... | git am ...' and the configuration variable `mailsplit.keepcr`.

Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
---
 t/t5101-mailinfo-dos.sh |   74 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 t/t5101-mailinfo-dos.sh
Show changes to t/t5101-mailinfo-dos.sh +74 −0
diff --git a/t/t5101-mailinfo-dos.sh b/t/t5101-mailinfo-dos.sh
new file mode 100644
index 0000000..c6acd55
--- /dev/null
+++ b/t/t5101-mailinfo-dos.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Stefan-W. Hahn
+#
+
+test_description='git-mailsplit mbox with dos line ending.
+
+'
+. ./test-lib.sh
+
+# Three patches which will be added as files with dos line ending.
+
+cat > file1 <<\EOF
+line 1
+EOF
+
+cat > file2 <<\EOF
+line 1
+line 2
+EOF
+
+cat > file3 <<\EOF
+line 1
+line 2
+line 3
+EOF
+
+test_expect_success 'setup repository with dos files' '
+        append_cr <file1 >file
+        git add file &&
+        git commit -m Initial &&
+        git tag start &&
+        append_cr <file2 >file
+        git commit -a -m Second &&
+        git tag start2 &&
+        append_cr <file3 >file
+        git commit -a -m Third &&
+	git format-patch -k --stdout start.. > format-patch.diff
+'
+
+test_expect_failure 'mailsplit format-patch of dos files' '
+        mkdir split &&
+        git mailsplit -osplit format-patch.diff &&
+        cat split/0001 split/0002 > mailsplit.diff &&
+        test_cmp format-patch.diff mailsplit.diff
+'
+
+test_expect_success 'mailsplit --keep-cr format-patch of dos files' '
+        mkdir split2 &&
+        git mailsplit --keep-cr -osplit2 format-patch.diff &&
+        cat split2/0001 split2/0002 > mailsplit2.diff &&
+        test_cmp format-patch.diff mailsplit2.diff
+'
+
+test_expect_success 'format-patch with dos files --keep-cr' '
+        git checkout -b new start &&
+	git format-patch -k --stdout start..master | git am --keep-cr -k -3 &&
+        git diff master
+'
+
+test_expect_success 'format-patch with dos files -e' '
+        git checkout -b new2 start &&
+	git format-patch -k --stdout start..master | git am -e -k -3 &&
+        git diff master
+'
+
+test_expect_success 'format-patch with dos files config.mailsplit' '
+        git config mailsplit.keepcr 1 &&
+        git checkout -b new3 start &&
+	git format-patch -k --stdout start..master | git am -k -3 &&
+        git diff master
+'
+
+test_done

-- 
1.7.0.rc2.17.g39385

← back to recent threads