Skip to contentSkip to conversation

git-send-mail in sh

DISCUSSION Andreas Ericsson 8 participants 22 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

Finally giving up on git-send-email (I won't install the 6 perl-modules it requires and I don't know perl enough to remove the need for them), I hacked up a replacement in sh. It's more aptly named as well. ;)

It tries to be fairly newbie-friendly in what it accepts so that new developers on a project easily can submit patches upstream in the desired format.

This is just a draft. If anyone thinks it's a good idea then say so and I'll write the man-page and re-submit it as a proper patch.

It's better than the perl version because;
1. It doesn't have any requirements other than normal unix-commands and "mail" being in the path.
2. It can generate the patches on the fly, using git-format-patch.

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).
2. The patches sent within the same second arrive in random order.

Sorry about the attachment btw. Thunderbird seems to wrap lines no matter what I tell it.

Thoughts? Comments?

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#1 lore raw
Quoted 8 lines · Andreas Ericsson

Finally giving up on git-send-email (I won't install the 6 perl-modules it requires and I don't know perl enough to remove the need for them), I hacked up a replacement in sh. It's more aptly named as well. ;)

It's better than the perl version because;
1. It doesn't have any requirements other than normal unix-commands and "mail" being in the path.
2. It can generate the patches on the fly, using git-format-patch.

Great!

Quoted 3 lines

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

Not so great. Why is it so much more difficult to have one more level of nesting? It's annoying, but it's a lot less annoying than having 19 separate threads that are all, in fact, related to each other.

Quoted 1 line

2. The patches sent within the same second arrive in random order.

Perhaps adding a 'sleep 1' would help? (The delay may be unacceptable to some people, though.)

nikolai

Signature · 3 lines
Nikolai Weibull: now available free of charge at http://bitwi.se/!
Born in Chicago, IL USA; currently residing in Gothenburg, Sweden.
main(){printf(&linux["\021%six\012\0"],(linux)["have"]+"fun"-97);}
#2 lore raw
Quoted 8 lines · Nikolai Weibull
Quoted 2 lines

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

Not so great. Why is it so much more difficult to have one more level of nesting? It's annoying, but it's a lot less annoying than having 19 separate threads that are all, in fact, related to each other.

I am of the opinion that nesting is bad because some patches get a few comments while some others get them in droves. It's easy to miss those with few if they're all nested. As for finding them, all the threads should show up next to each other since there's practically no delay between sending them.

As for implementation, I don't think most "mail" programs have the functionality necessary to do so (dunno though since I didn't investigate).

Quoted 7 lines
Quoted 2 lines

2. The patches sent within the same second arrive in random order.

Perhaps adding a 'sleep 1' would help? (The delay may be unacceptable to some people, though.)

I thought about that, but decided against it.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#3 lore raw

On Fri, Nov 25, 2005 at 11:59:07AM +0100, Andreas Ericsson wrote:
| As for implementation, I don't think most "mail" programs have the | functionality necessary to do so (dunno though since I didn't investigate).

You can always generate a 'valid' mail message and use the sendmail binary directly. That way you can set proper Message-Id: and proper
References: and/or In-Reply-To: in the following mails.

Cheers,
Ferdy

Signature · 3 lines
Fernando J. Pereda Garcimartín
Gentoo Developer (Alpha,net-mail,mutt,git)
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4
#4 lore raw
In reply to Andreas Ericsson
Quoted 3 lines · Andreas Ericsson

Finally giving up on git-send-email (I won't install the 6 perl-modules it requires and I don't know perl enough to remove the need for them), I hacked up a replacement in sh. It's more aptly named as well. ;)

Scanning the list, 2 are related to option handling (one of which is builtin), one isn't used (Data::Dumper), and two are related to sending valid emails. The email address verification is ridiculously hard to get correct, so using pre-written code for that seemed justified.

Quoted 3 lines

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

You can use --no-chain-reply-to in git-send-email.perl, and put a 0/N message in, and all subsequent replies get attached to that instead of in order, if you want. This keeps everything in one thread, but all as responses to the first email, so the people that want everything in one thread can get that behavior.

Signature · 2 lines
Ryan Anderson
  sometimes Pug Majere
#5 lore raw
Quoted 8 lines · Ryan Anderson
Quoted 1 line · Andreas Ericsson

Finally giving up on git-send-email (I won't install the 6 perl-modules it requires and I don't know perl enough to remove the need for them), I hacked up a replacement in sh. It's more aptly named as well. ;)

Scanning the list, 2 are related to option handling (one of which is builtin), one isn't used (Data::Dumper), and two are related to sending valid emails.

When I try to install Email::Valid (using apt) it wants an additional two modules. Mail::Sendmail wants one other, so that's Data::Dumper, the two actually used and the three those two use. Six, for short.

Quoted 3 lines

The email address verification is ridiculously hard to get correct, so using pre-written code for that seemed justified.

But it isn't necessary to validate it to such exactness. Nothing worse will happen than the user chiding himself for his butterfingers if he/she makes a mistake.

Besides, I think typos are by far the most common error. Those are usually valid email addresses while still not being correct.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#6 lore raw
Quoted 9 lines · Andreas Ericsson
Quoted 7 lines · Ryan Anderson
Quoted 4 lines · Andreas Ericsson

Finally giving up on git-send-email (I won't install the 6 perl-modules it requires and I don't know perl enough to remove the need for them), I hacked up a replacement in sh. It's more aptly named as well. ;)

Quoted 7 lines
Quoted 3 lines

Scanning the list, 2 are related to option handling (one of which is builtin), one isn't used (Data::Dumper), and two are related to sending valid emails.

When I try to install Email::Valid (using apt) it wants an additional two modules. Mail::Sendmail wants one other, so that's Data::Dumper, the two actually used and the three those two use. Six, for short.

Can I ask why you aren't willing to install packages, such as those? I can understand a reluctance to install modules directly from CPAN, on an otherwise package-managed system, but I'm afraid I must confess to puzzlement over a reluctance to use pre-packaged modules.

The major flaw in git-send-email, from my perspective, was a lack of support for SMTP AUTH, for situations like Junio's, where the local MTA (and thus "mail" as well) are not configured to handle SMTP AUTH. Moving to a purely shell based replacement seems to make this an even harder feature to support. (Though, admittedly, I haven't even made an attempt to add it to the Perl version yet.)

Quoted 10 lines
Quoted 3 lines

The email address verification is ridiculously hard to get correct, so using pre-written code for that seemed justified.

But it isn't necessary to validate it to such exactness. Nothing worse will happen than the user chiding himself for his butterfingers if he/she makes a mistake.

Besides, I think typos are by far the most common error. Those are usually valid email addresses while still not being correct.

Fair enough.

#7 lore raw
Quoted 7 lines · Ryan Anderson
Quoted 3 lines · Andreas Ericsson

When I try to install Email::Valid (using apt) it wants an additional two modules. Mail::Sendmail wants one other, so that's Data::Dumper, the two actually used and the three those two use. Six, for short.

Quoted 5 lines

Can I ask why you aren't willing to install packages, such as those? I can understand a reluctance to install modules directly from CPAN, on an otherwise package-managed system, but I'm afraid I must confess to puzzlement over a reluctance to use pre-packaged modules.

I don't like having lots of junk installed. Besides, I do a lot of development work for the Openwall distro which tries fairly hard to get away without installing lots of cruft. I'd rather not taint it with packages from other vendors since I do a fair amount of RPM building and testing on it but I still want to be able to use git on it.

Quoted 7 lines

The major flaw in git-send-email, from my perspective, was a lack of support for SMTP AUTH, for situations like Junio's, where the local MTA (and thus "mail" as well) are not configured to handle SMTP AUTH. Moving to a purely shell based replacement seems to make this an even harder feature to support. (Though, admittedly, I haven't even made an attempt to add it to the Perl version yet.)

By "local" do you mean "local on Junio's laptop" or "local at cox.net"?

"mail" uses the "local on Junio's laptop" SMTP server so he can configure it any way he wants.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#8 lore raw
Quoted 4 lines · Andreas Ericsson

By "local" do you mean "local on Junio's laptop" or "local at cox.net"?

"mail" uses the "local on Junio's laptop" SMTP server so he can configure it any way he wants.

I am puzzled. What if I do not run any SMTP server on the laptop and use ISP's SMTP server? Right now my ISP's SMTP server does not seem to require AUTH, so it is not an issue for me, though..

#9 lore raw
Quoted 5 lines · Junio C Hamano
Quoted 2 lines · Andreas Ericsson

By "local" do you mean "local on Junio's laptop" or "local at cox.net"?

Quoted 8 lines
Quoted 1 line

"mail" uses the "local on Junio's laptop" SMTP server so he can configure it any way he wants.

I am puzzled. What if I do not run any SMTP server on the laptop and use ISP's SMTP server? Right now my ISP's SMTP server does not seem to require AUTH, so it is not an issue for me, though..

It uses whatever the /bin/mail program on your system uses. This is usually done by spooling the mail for delivery by the local MTA which doesn't have to listen to any ports anywhere (mutt and friends work the same way).

Having an MTA installed is a requirement of the LSB. The /bin/mail program requires that it's running, which the sendmail binary doesn't. The sendmail binary is always shipped along with an MTA though, so to get around having one at all one would have to re-implement the SMTP protocol (which Mail::Sendmail does, but without authentication). I can do that in C if you like. That way you can have support for SMTP over SSL with all sorts of funny authentication mechanisms.

The good thing about using the local MTA is that you get that for free with very thoroughly tested code and you only have to set it up once rather than passing all the auth stuff repeatedly on the command-line each time you want to submit a patch.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#10 lore raw
In reply to Andreas Ericsson
Quoted 1 line · Andreas Ericsson

It's better than the perl version because;

Good.

Quoted 3 lines

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

I think that is an improvement, actually ;-)

Quoted 1 line

2. The patches sent within the same second arrive in random order.

I think you can fudge the "Date: " yourself. Count the number of messages you are going to send out, grab the wallclock time before starting to send the first message, subtract that number of seconds and give it to the first message, add 1 second and give it to the second message, and so on.

3. It does not CC signers and authors. Although I personally consider not doing it "better", some people _might_ want to keep that behaviour as an option.

Quoted 4 lines

# Instead of applying the 8942 chars long RFC-exact regex to

# match recipients email addresses against, we're satisfied with
# a simple @ somewhere inside an argument and just assume that
# people won't try anything obviously stupid

This is probably adequate in practice. I have not seen an e-mail address other than local-part@domain (RFC2822-speak "addr-spec") form of mailbox on the kernel list for some time.

Quoted 4 lines

function usage() {
echo "Usage: git submit [email protected] <commit-ish> [<commit-ish>]" exit 1
}

I'm old fashioned and tend to omit noise word "function".

The original format-patch parameters are my fault, but I'd rather see newly written commands done like this:

	"git-send-email" <param>+

        <param> = <patch> | <addressee> | <commits>
        <patch> = <anything that passes "test -f">
	<addressee> = <RFC2822 addr-spec>
        <commits> = ".." <top> | <bottom> ".." <top> | <commit>
	<bottom> = <extended SHA1 expression>
	<top>    = <extended SHA1 expression>
	<commit> = <extended SHA1 expression>

* ..<top> is a shorthand of "origin"..<top> (the choice of "origin" might be debatable, but probably sane).

* <bottom>..<top> pair is to format changes in <top> but not in <bottom>; typically <top> is the name of a topic branch, and <bottom> is typically "origin". This is to encourage the use of topic branches.

* <commit> is a shorthand for <commit>^1..<commit>; this is to allow you to quickly pick just one commit and send it out.

Quoted 4 lines

function abort() {
echo "Aborting."
exit 0
}

Abort but exit 0? You do not seem to be using it though ;-).

Quoted 9 lines

commits=0
if [ "$com1" ]; then

	if [ -z "$com2" ]; then
		com2="$com1"
		com1=HEAD
	fi

	commits=$(git rev-list $com1 ^$com2 | wc -l)

fi

You do not want to count commits like this. format-patch drops patches that are already in upstream even if they are recorded as diffrent commit objects, so the number you get from rev-list is just an upper bound, and may not match the number of commits that would be formatted.

Quoted 1 line

[ $commits -eq 0 -a -z "$patches" ] && usage

And I'd probably drop this one as well; you can have the check before sending things out, right?

Quoted 1 line

# [ "$email" ] || git repo-config --get patch_email_address

Storing the default addressee in the config is a good idea, since typically e-mail submissions are to a single address.

Quoted 1 line

[ $commits -gt 1 ] && opts=-n

You can always say -n if you want to do this; format-patch -n with a single patch would not say [PATCH 1/1].

Quoted 3 lines

for patch in $(git format-patch $opts $com2 $com1 | sed 's/^* //'); do
patches="$patches $patch"
done

This is the first script I saw that uses the standard output from format-patch, and I do not think nobody else used it so far. If the standard output from format-patch is useful like this, I would like to drop the '* ' prefix from it, so that you do not have to sed it out.

You would probably want to do "format-patch -o $tmpdir" at least not to smudge the toplevel directory.

#11 lore raw
Quoted 6 lines · Junio C Hamano
Quoted 4 lines

function usage() {
echo "Usage: git submit [email protected] <commit-ish> [<commit-ish>]" exit 1
}

I'm old fashioned and tend to omit noise word "function".

More importantly, it is not portable.

Signature · 4 lines
Yann Dirson    <[email protected]> |
Debian-related: <[email protected]> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>
#12 lore raw
In reply to Junio C Hamano
Quoted 8 lines · Junio C Hamano
Quoted 2 lines · Andreas Ericsson

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

I think that is an improvement, actually ;-)

Agreed to that then. Good thing since that was the hardest to solve.

Quoted 10 lines
Quoted 2 lines

2. The patches sent within the same second arrive in random order.

I think you can fudge the "Date: " yourself. Count the number of messages you are going to send out, grab the wallclock time before starting to send the first message, subtract that number of seconds and give it to the first message, add 1 second and give it to the second message, and so on.

Quoted 4 lines

3. It does not CC signers and authors. Although I personally consider not doing it "better", some people _might_ want to keep that behaviour as an option.

It doesn't CC them, but any number of email-addresses can be specified on the command line (so long as they don't include spaces, but that can be taken care of).

These below needs a bit of clarification. It's friday afternoon here, so I'm a bit slow.

Quoted 8 lines

<commits> = ".." <top> | <bottom> ".." <top> | <commit> <bottom> = <extended SHA1 expression>
<top> = <extended SHA1 expression>
<commit> = <extended SHA1 expression>

* ..<top> is a shorthand of "origin"..<top> (the choice of "origin" might be debatable, but probably sane).

I'd rather specify the entry-point, as in "get all patches from this commit to HEAD", if only one commit is specified, so:

git-send-patch [email protected] origin

would do just that.

Quoted 5 lines

* <bottom>..<top> pair is to format changes in <top> but not in <bottom>; typically <top> is the name of a topic branch, and <bottom> is typically "origin". This is to encourage the use of topic branches.

Would that be

git-send-patch origin..HEAD

to get the changes in the current branch since head?

Quoted 3 lines

* <commit> is a shorthand for <commit>^1..<commit>; this is to allow you to quickly pick just one commit and send it out.

Marvellous the things one learn. I didn't know about that syntax before. :)

Quoted 7 lines
Quoted 2 lines

# [ "$email" ] || git repo-config --get patch_email_address

Storing the default addressee in the config is a good idea, since typically e-mail submissions are to a single address.

If values can have spaces there can be any number of email-addresses.

Quoted 7 lines
Quoted 2 lines

[ $commits -gt 1 ] && opts=-n

You can always say -n if you want to do this; format-patch -n with a single patch would not say [PATCH 1/1].

Didn't know that. Good thing though.

Quoted 7 lines

This is the first script I saw that uses the standard output from format-patch, and I do not think nobody else used it so far. If the standard output from format-patch is useful like this, I would like to drop the '* ' prefix from it, so that you do not have to sed it out.

I'll do that then. It doesn't really add any value anyways.

Quoted 3 lines

You would probably want to do "format-patch -o $tmpdir" at least not to smudge the toplevel directory.

Perhaps support the -o flag in git-send-patch?

I'm wondering if it wouldn't be better to move much of git-format-patch's functionality to git-send-patch and support a "--todisk" option. After all, how many patches are created but not sent anywhere?

That way we could rework the syntax to only support that of git-rev-list. I think it's the most standard-like thing there is in git.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#13 lore raw

Subject: [PATCH] format-patch: output filename reported to stdout verbatim.

Prepending asterisk to the output was just adding noise, and forcing scripts like git-send-mail proposed by Andreas Ericsson do unnecessary work.

---

Quoted 7 lines · Andreas Ericsson
Quoted 3 lines · Junio C Hamano

... If the standard output from format-patch is useful like this, I would like to drop the '* ' prefix from it, so that you do not have to sed it out.

I'll do that then. It doesn't really add any value anyways.

Agreed, so I'll push this out in the next batch.

 git-format-patch.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

applies-to: 9ccf8849fa9b522a344645c2f28f12ab036e30d5 51b3c00e9d95371a9ad202204f01c5981f241b20

git-format-patch.sh +2 -2
index bc56876..9b40880 100755
@@ -268,7 +268,7 @@ do
     file=`printf '%04d-%stxt' $i "$title"`
     if test '' = "$stdout"
     then
-	    echo "* $file"
+	    echo "$file"
 	    process_one >"$outdir$file"
 	    if test t = "$check"
 	    then
@@ -279,7 +279,7 @@ do
 		:
 	    fi
     else
-	    echo >&2 "* $file"
+	    echo >&2 "$file"
 	    process_one
     fi
     i=`expr "$i" + 1`

---
@@GIT_VERSION@@

#14 lore raw
In reply to Andreas Ericsson
Quoted 3 lines · Andreas Ericsson

It doesn't CC them, but any number of email-addresses can be specified on the command line (so long as they don't include spaces, but that can be taken care of).

Again I do not think I'd ever use that feature from the original send-email myself, but the difference is that this CC list depends on each commit (sign-offs taken from a commit are added to CC list for only that commit).

Quoted 10 lines
Quoted 4 lines

* <bottom>..<top> pair is to format changes in <top> but not in <bottom>; typically <top> is the name of a topic branch, and <bottom> is typically "origin". This is to encourage the use of topic branches.

Would that be

git-send-patch origin..HEAD

to get the changes in the current branch since head?

Yes, and that could be spelled "git-send-patch ..HEAD" as well, if we go with my suggestion to default <bottom> to "origin".

Quoted 4 lines
Quoted 2 lines

* <commit> is a shorthand for <commit>^1..<commit>; this is to allow you to quickly pick just one commit and send it out.

Marvellous the things one learn. I didn't know about that syntax before. :)

Just to make sure you did not misunderstand me, I meant: the proposed program acts as if <commit>^1..<commit> was given when single <commit> is given.

But you are right. We could make a single <commit> a short-hand for "origin"..<commit>; if somebody wants to pick just one commit from a topic branch, he can always say <commit>^1..<commit>.

Quoted 4 lines

I'm wondering if it wouldn't be better to move much of git-format-patch's functionality to git-send-patch and support a "--todisk" option. After all, how many patches are created but not sent anywhere?

Manymanymanymanymany. I do all my rebases and cherry-picks via format-patch piped to git-am, and obviously they are never sent out.

#15 lore raw
Quoted 11 lines · Junio C Hamano
Quoted 1 line · Andreas Ericsson

It doesn't CC them, but any number of email-addresses can be specified on the command line (so long as they don't include spaces, but that can be taken care of).

Again I do not think I'd ever use that feature from the original send-email myself, but the difference is that this CC list depends on each commit (sign-offs taken from a commit are added to CC list for only that commit).

Had a thinko when I wrote that. I've added --cc-signers, --cc-author and --cc (for both --cc-signers and --cc-author).

Quoted 2 lines

But you are right. We could make a single <commit> a short-hand for "origin"..<commit>;

Actually, I meant that a single <commit> would mean "<commit>..HEAD", like git-format-patch does it. Doing the other way around in a tool so closely coupled would be very confusing, I think.

Here's what I have on disk right now. The ${var##*^} syntax was decided to be portable in some earlier discussion, so I'm sticking with it (mostly because I don't know how to do it with expr and Junio pokes me when I do it with sed. Enlightenment welcome).

if [ "$com2" ]; then

    range="$com1..$com2"
else
    case "$com1" in
        ?*..?*)
            # nicely ranged already
            range="$com1"
            ;;
        ..)
            range=origin..HEAD
            ;;
        ?*^)
            # single commit
            com1="${com1##*^}"
            range="$com1^1..$com1"
            ;;
        ?*^[0-9]|?*^[0-9][0-9])
            # series of commits, ranging back from <commit-ish>
            range="$com1..${com1%%^*}"
            ;;
        ^[0-9]|^[0-9][0-9])
            # series of commits, ranging back from HEAD
            range="HEAD$com1..HEAD"
            ;;
        *)
            range="$com1..HEAD"
            ;;
    esac

fi

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#16 lore raw
Quoted 4 lines · Andreas Ericsson

Here's what I have on disk right now. The ${var##*^} syntax was decided to be portable in some earlier discussion, so I'm sticking with it (mostly because I don't know how to do it with expr and Junio pokes me when I do it with sed. Enlightenment welcome).

The ${parameter##word} syntax is in IEEE 1003.1-2001, and bash, ksh, and dash seem to work with it. That does not necessarily mean it is "portable" but I won't be so worried about shells that do not grok this. Input from people on non-Linux platforms are appreciated.

Quoted 10 lines

if [ "$com2" ]; then

    range="$com1..$com2"
else
    case "$com1" in
...
        ?*^)
            # single commit
            com1="${com1##*^}"
            range="$com1^1..$com1"
            ;;

I wonder if you meant "${com1%^}" here, to remove the trailing '^'.

Quoted 8 lines
        ?*^[0-9]|?*^[0-9][0-9])
            # series of commits, ranging back from <commit-ish>
            range="$com1..${com1%%^*}"
            ;;
        ^[0-9]|^[0-9][0-9])
            # series of commits, ranging back from HEAD
            range="HEAD$com1..HEAD"
            ;;

N generation back in extended SHA1 notation uses a tilde '~', e.g. "HEAD~5" is five commits back from the current HEAD, so I'd prefer being consistent with that (HEAD^5 means the fifth parent of an octopus merge commit). Also limiting to between 0 and 99 generations misinterprets "HEAD~123".

Although checking only the letter that follows the tilde is a digit mistakenly accepts something like "master~1-bad-one", that is already malformed and whatever comes downstream would barf, so that may be fine. How about something like:

	?*'~'[1-9]*)
        	range="$com1..${com1%~*}" ;;
	'~'[1-9]*)
        	range="HEAD$com1..HEAD" ;;

I do not have aversion against echo piped to sed in general, by the way. I *would* redicule people who write something like this, though:

	case "$git" in
        */.git)	printname=`echo "$git" | sed -e 's/\/\.git$//'` ;;
        *)	printname=$git ;;
	esac

It should be spelled expr "$git" : '\(.*\)/\.git$' (or "${git%/.git}" if we know the shell is POSIX), for this particular one, since we already know it ends with "/.git". But if all you want to do is to drop an *optional* trailing "/.git" (i.e. your input may or may not end with "/.git"), a single:

	printname=`echo "$git" | sed -e 's/\/\.git$//'`

without surrounding case may be adequate; it forks sed when it does not have the optional /.git part, though. And if you are dropping optional /.git or .git (think of prettyprinting uemacs/.git and git.git), then echo-to-sed without surrounding "case" is probably easier to read:

	printname=`echo "$git" | sed -e 's/\/*\.git$//'`

unless you want to avoid fork, in which case it would be:

	case "$git" in
        */.git) printname=${git%/.git} ;;
        *.git) printname=${git%.git} ;;
        *) printname=$git ;;
	esac
#17 lore raw
Quoted 2 lines · Junio C Hamano

The ${parameter##word} syntax is in IEEE 1003.1-2001, and bash, ksh, and dash seem to work with it.

You can add Zsh to that list.

nikolai

Signature · 3 lines
Nikolai Weibull: now available free of charge at http://bitwi.se/!
Born in Chicago, IL USA; currently residing in Gothenburg, Sweden.
main(){printf(&linux["\021%six\012\0"],(linux)["have"]+"fun"-97);}
#18 lore raw
In reply to Junio C Hamano
Quoted 10 lines · Junio C Hamano
Quoted 4 lines · Andreas Ericsson
           # single commit
           com1="${com1##*^}"
           range="$com1^1..$com1"
           ;;

I wonder if you meant "${com1%^}" here, to remove the trailing '^'.

I did/do/done. :)

Quoted 11 lines
Quoted 9 lines
       ?*^[0-9]|?*^[0-9][0-9])
           # series of commits, ranging back from <commit-ish>
           range="$com1..${com1%%^*}"
           ;;
       ^[0-9]|^[0-9][0-9])
           # series of commits, ranging back from HEAD
           range="HEAD$com1..HEAD"
           ;;
Quoted 1 line

N generation back in extended SHA1 notation uses a tilde '~',

I just noticed that after sending the original email. I've changed it to take tilde instead.

Quoted 8 lines

Also limiting to between 0 and 99
generations misinterprets "HEAD~123".

Although checking only the letter that follows the tilde is a digit mistakenly accepts something like "master~1-bad-one", that is already malformed and whatever comes downstream would barf, so that may be fine. How about something like:

Quoted 5 lines
	?*'~'[1-9]*)
        	range="$com1..${com1%~*}" ;;
	'~'[1-9]*)
        	range="HEAD$com1..HEAD" ;;

Fine by me, although that 99 patches limit was sort of semi-intentional. Doing it this way makes case order matter since

	?*'~'[1-9]*

will also match

	<commit>~3..HEAD

I'll stick with your way though and put a comment there so people don't touch the ordering.

Thanks for the expr lesson btw.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#19 lore raw
In reply to Andreas Ericsson

Hi,

Quoted 3 lines · Andreas Ericsson

It's worse than the perl version because;
1. It doesn't thread the patch-series (which I personally prefer anyway since it's easier to follow a thread on a particular patch that way).

I think you can do that easily by providing a Message-ID: and a
References: header. The id could be made up by "git-$commit_id" to be reasonably unique.

Quoted 1 line

2. The patches sent within the same second arrive in random order.

I have that all the time. Sometimes, I send emails to the git list several minutes apart, and they come out in the wrong order. So it is no problem.

Quoted 2 lines

Sorry about the attachment btw. Thunderbird seems to wrap lines no matter what I tell it.

The hints in SubmittingPatches did not help?

Quoted 1 line

Thoughts? Comments?

I find it very cool. And easy to read. Just a few nits: You could use git-sh-setup.sh to ensure that you're in a valid git repository. Also, you could reuse the "die" function contained therein instead of a new function, "abort".

Ciao,
Dscho

#20 lore raw
Quoted 6 lines · Johannes Schindelin
Quoted 2 lines

Sorry about the attachment btw. Thunderbird seems to wrap lines no matter what I tell it.

The hints in SubmittingPatches did not help?

Nopes. Perhaps because I started editing the message before I changed the settings. I'll investigate further and make amendments if necessary.

Quoted 6 lines
Quoted 2 lines

Thoughts? Comments?

I find it very cool. And easy to read. Just a few nits: You could use git-sh-setup.sh to ensure that you're in a valid git repository. Also, you could reuse the "die" function contained therein instead of a new function, "abort".

Will do. Thanks for the feedback.

Signature · 3 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
#21 lore raw
Quoted 1 line · Andreas Ericsson

Johannes Schindelin wrote:

...

Quoted 4 lines
Quoted 2 lines

The hints in SubmittingPatches did not help?

Nopes. Perhaps because I started editing the message before I changed the settings. I'll investigate further and make amendments if necessary.

Definitely need to do the settings changes *before* opening the compose window for them to have an effect.