threads / patch / 38354

patchformat-patch: print format-patch usage if there are no arguments

Subject: [PATCH] format-patch: print format-patch usage if there are no arguments

## tl;dr

10 messages between Jan 13, 2015 and Jan 14, 2015. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Alexander Kuleshov· Jan 13, 2015, 17:54 UTC · lore
Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 builtin/log.c | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to builtin/log.c +4 −0
diff --git a/builtin/log.c b/builtin/log.c
index ad3cfd8..4431b50 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1246,6 +1246,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	/* We need at least one revision */
+	if (argc == 1)
+		usage_with_options(builtin_format_patch_usage, builtin_format_patch_options);
+
 	extra_hdr.strdup_strings = 1;
 	extra_to.strdup_strings = 1;
 	extra_cc.strdup_strings = 1;
-- 
2.3.0.rc0.239.g0ae1f56.dirty
Alexander Kuleshov· Jan 13, 2015, 18:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

Hello Junio,

As some commands does it when they are executed without arguments, like git config, git blame and etc...

2015-01-14 0:43 GMT+06:00 Junio C Hamano <gitster@pobox.com>:
> Why?
-- 
_________________________
0xAX
Junio C Hamano· Jan 13, 2015, 19:17 UTC · re: Alexander Kuleshov · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

Alexander Kuleshov <kuleshovmail@gmail.com> writes:
Show 5 quoted lines
> 2015-01-14 0:43 GMT+06:00 Junio C Hamano <gitster@pobox.com>:
>> Why?
>
> As some commands does it when they are executed without arguments,
> like git config, git blame and etc...

For format-patch, I think the current behaviour is more of the lack of implementation of the obvious default. "git blame" does not have any obvious default (would there be a single file you would want to dig the history when the user does not tell you which one? No), so it proabaly is the right thing to do to error out.

But I would not surprised if those who build on top of others' work to wish that "git format-patch" that was invoked without argument on a branch created by "git checkout -t -b" to default to format commits since the branch forked from @{upstream}, for example.

Stefan Beller· Jan 13, 2015, 20:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

On Tue, Jan 13, 2015 at 11:17 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Alexander Kuleshov <kuleshovmail@gmail.com> writes:
>
>> 2015-01-14 0:43 GMT+06:00 Junio C Hamano <gitster@pobox.com>:
>>> Why?
>>
>> As some commands does it when they are executed without arguments,
>> like git config, git blame and etc...
>
> For format-patch, I think the current behaviour is more of the lack
> of implementation of the obvious default.  "git blame" does not have
> any obvious default (would there be a single file you would want to
> dig the history when the user does not tell you which one?  No), so
> it proabaly is the right thing to do to error out.
>
> But I would not surprised if those who build on top of others' work
> to wish that "git format-patch" that was invoked without argument on
> a branch created by "git checkout -t -b" to default to format commits
> since the branch forked from @{upstream}, for example.

My initial reaction was to assume HEAD^ if no arguments are given to format-patch. Though after thinking about it, the behavior you're proposing makes more sense.

Another idea would be to take the first commit which is pointed to by another branch as the first commit in the commit range. I am currently thinking about the atomic patch series which I developed on top of origin/mh/reflog-expire and that would be the first branch you find if you just go back in history and look for any branches. Usually that would be the upstream tracking branch though, so this would be an extension to your proposal.

Though I am not sure if this scales. (Who has many branches anyway? ;)
Junio C Hamano· Jan 13, 2015, 22:28 UTC · re: Stefan Beller · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

Stefan Beller <sbeller@google.com> writes:
> Another idea would be to take the first commit which is pointed to by
> another branch as the first commit in the commit range.

Trying to figure out what happened from the topology of the history is certainly attractive proposition, but I suspect that it would be too fragile to be practically useful. The other topic you may have based your work on may have advanced independently (e.g. a "oops, I forgot this one" bugfix), resulting in a fork like this:

 ---o---o---o---o (fixed reflog-expire)
             \
              x---x---x (atomic-push)

and the fork-point is no longer at the tip of anything. "Excluding anything that is on another branch" would cover the forked case much better, but that is only true if there is no integration branch like 'pu' or 'next', in which case, only an early part of atomic-push may already be part of another branch 'next' while the remainder is not, or the entirety of atomic-push may be part of another branch 'pu'.

On the other hand, "I am forked from building on this one" done with "checkout -t" is an explicit mark the user leaves, so it would serve as a better hint to base the default heuristics on, I think.

But nobody is asking for such a feature ;-)
Jeff King· Jan 13, 2015, 22:45 UTC · re: Junio C Hamano · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

On Tue, Jan 13, 2015 at 02:28:13PM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> On the other hand, "I am forked from building on this one" done with
> "checkout -t" is an explicit mark the user leaves, so it would serve
> as a better hint to base the default heuristics on, I think.
> 
> But nobody is asking for such a feature ;-)

FWIW, I very rarely run format-patch directly, but have a wrapper script that dumps the patches into a tempfile and runs mutt. I taught it in 2007 to use the upstream branch as the default[1], and was puzzled reading the start of this thread, thinking we already did that.

So that is perhaps not asking for the feature (I am already happy with my homegrown wrapper), but is maybe an endorsement of it. :)

-Peff
[1] You may note in 2007 that we did not even have @{upstream}. I
    implemented it manually using git-config! Then in 2009, I switched
    it to use for-each-ref's "%(upstream)" placeholder. Literally 5 days
    later, Dscho introduced @{upstream}, but I never got around to
    switching. Maybe now it is time. :)
Stefan Beller· Jan 13, 2015, 22:48 UTC · re: Jeff King · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

On Tue, Jan 13, 2015 at 2:45 PM, Jeff King <peff@peff.net> wrote:
Show 11 quoted lines
> On Tue, Jan 13, 2015 at 02:28:13PM -0800, Junio C Hamano wrote:
>
>> On the other hand, "I am forked from building on this one" done with
>> "checkout -t" is an explicit mark the user leaves, so it would serve
>> as a better hint to base the default heuristics on, I think.
>>
>> But nobody is asking for such a feature ;-)
>
> So that is perhaps not asking for the feature (I am already happy with
> my homegrown wrapper), but is maybe an endorsement of it. :)
>

Would you mind to share (parts of) the wrapper script? We could see if that makes sense to incorporate into format-patch as well.

Thanks, Stefan

Jeff King· Jan 14, 2015, 01:28 UTC · re: Stefan Beller · lore

my hacky mutt-specific format-patch workflow

[retitling in case there is a wider audience]
On Tue, Jan 13, 2015 at 02:48:15PM -0800, Stefan Beller wrote:
> Would you mind to share (parts of) the wrapper script? We could see if that
> makes sense to incorporate into format-patch as well.
Sure, but I'll warn you it's kind of gross and specific to my workflow. :)

The script (which I call mpatch) is at the end of this message. The main thing which may be of interest to other people is the cover-letter handling. I write my cover letters in my normal MUA (mutt), and then generate the series as a reply to that. The two ways it supports that are:

  1. It generates the list of patches that I include in my cover letter
     from the mbox.  This _should_ be a "git log" one-liner, but our
     --pretty placeholders do not know how to count. Ideally I could say
     "[%i/%n]: %s", but neither of those first two exist (and %n would
     require counting all of the commits first, which might require some
     surgery to git-log).
  2. Given an existing message, it will pick out the to, cc, and
     message-id headers, and generate corresponding --to, --cc, and
     --in-reply-to arguments to feed to format-patch. I do it hackily in
     perl, but probably format-patch could do it internally if it built
     on git-mailinfo.

I typically have my MUA in one terminal and a shell running git in the other. Even though I could run mpatch directly from the MUA, it is often missing the context of which repo I am working in. So instead, I typically use ~/patch as a go-between for the two sessions (both for generating patch series, but also for applying with git-am). So I have this in my muttrc:

  macro pager,index D '<shell-escape>rm -f $HOME/patch<enter>'
  macro pager,index A '<copy-message>~/patch<enter><enter>'

Applying a patch series from the list is just 'D' to clear the state, then 'A' to collect whatever patches. And then I "git am ~/patch" from the terminal window.

Generating a patch series is more like:
  1. "mpatch >~/patch" from the git terminal to generate the list of
      commits. No arguments necessary, because it uses @{upstream} as
      the base (but occasionally I use "-3" or similar if I am sending
      out new patches on top of somebody else's work).
  2. Reply or start a new thread in mutt, as normal. This becomes the
     cover letter (the to/cc comes from the reply, or I have mutt
     aliases for the list and frequent contributors, and/or I may cut
     and paste from "git log" in some cases).  Mutt dumps me in vim to
     write the actual message, and I ":r ~/patch" to pull it in.
  3. Finish and send off the cover letter. This gives it a message-id.
  4. Drop the newly-sent message into ~/patch. I usually just open my
     sent folder and use 'D', 'A' to copy it there.
  5. "mpatch" from the git terminal. The headers are picked up from
     ~/patch. Sometimes I use "-v2", which is passed to format-patch
     to get "[PATCH v2 i/n]".

After that, I'm in mutt with an mbox full of the patches. I have this hotkey in mutt:

  macro index,pager b ":set edit_headers=yes<enter><resend-message>:set edit_headers=no<enter>"

which dumps me vim, with the headers. From there I give a final proofread, write any comments below the "---", and then send it.

I imagine for the last bit many people have a similar workflow without mutt that is something like:

  1. format-patch into ~/patch/*
  2. $EDITOR ~/patch/*
  3. git send-email ~/patch/*

Those people would potentially benefit from the format-patch in step 1 picking up the headers from an existing message.

-Peff

-- >8 -- #!/bin/sh

upstream_branch() {
  current=`git symbolic-ref HEAD`
  upstream=`git for-each-ref --format='%(upstream)' "$current"`
  if test -n "$upstream"; then
    echo $upstream
  else
    echo origin
  fi
}
get_reply_headers() {
  perl -ne '
    if (defined $opt && /^\s+(.*)/) {
      $val .= " $1";
      next;
    }
    if (defined $opt) {
      print "--$opt=", quotemeta($val), " ";
      $opt = $val = undef;
    }
    if (/^(cc|to):\s*(.*)/i) {
      $opt = lc($1);
      $val = $2;
    }
    elsif (/^message-id:\s*(.*)/i) {
      $opt = "in-reply-to";
      $val = $1;
    }
  '
}
has_nonoption=
for i in "$@"; do
  case "$i" in
    -[0-9]) has_nonoption=yes ;;
    -*) ;;
     *) has_nonoption=yes
  esac
done

: ${REPLY:=$HOME/patch} test -n "$REPLY" && eval "set -- `get_reply_headers <\"$REPLY\"` \"\$@\"" test "$has_nonoption" = "yes" || set -- "$@" `upstream_branch`

git format-patch -s --stdout --from "$@" >.mbox
if test -t 1; then
  mutt -f .mbox
else
  perl -lne '
    if (/^Subject: (.*)/) {
      $subject = $1;
    }
    elsif ($subject && /^\s+(.*)/) {
      $subject .= " $1";
    }
    elsif ($subject) {
      print $subject;
      $subject = undef;
    }
  ' .mbox |
  sed -e 's/\[PATCH /[/' \
      -e 's/]/]:/' \
      -e 's/^/  /'
fi
rm -f .mbox
Junio C Hamano· Jan 13, 2015, 23:41 UTC · re: Jeff King · lore

Re: [PATCH] format-patch: print format-patch usage if there are no arguments

Jeff King <peff@peff.net> writes:
> So that is perhaps not asking for the feature (I am already happy with
> my homegrown wrapper), but is maybe an endorsement of it. :)

OK. A patch to add this should be reasonably clean and trivial, I would guess.

← back to recent threads