Patchwork [3,of,3] patchbomb: make --git-format-patch imply --plain

login
register
mail settings
Submitter Henning Schild
Date Nov. 17, 2016, 8:47 p.m.
Message ID <de2b03a509491020f728.1479415678@localhost>
Download mbox | patch
Permalink /patch/17630/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Henning Schild - Nov. 17, 2016, 8:47 p.m.
# HG changeset patch
# User Henning Schild <henning@hennsch.de>
# Date 1479415557 -3600
#      Thu Nov 17 21:45:57 2016 +0100
# Node ID de2b03a509491020f728f1955e39e2bfb9a77426
# Parent  a9be53e26cb1ac19d1c0156062e8ae23f8366d8b
patchbomb: make --git-format-patch imply --plain

Not using --plain would generate mails with the hg header in the git commit
message. Since --git-format-patch already caters for git, might as well make
sure users do not forget --plain.
Yuya Nishihara - Nov. 20, 2016, 10:38 a.m.
On Thu, 17 Nov 2016 21:47:58 +0100, Henning Schild wrote:
> # HG changeset patch
> # User Henning Schild <henning@hennsch.de>
> # Date 1479415557 -3600
> #      Thu Nov 17 21:45:57 2016 +0100
> # Node ID de2b03a509491020f728f1955e39e2bfb9a77426
> # Parent  a9be53e26cb1ac19d1c0156062e8ae23f8366d8b
> patchbomb: make --git-format-patch imply --plain
> 
> Not using --plain would generate mails with the hg header in the git commit
> message. Since --git-format-patch already caters for git, might as well make
> sure users do not forget --plain.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -418,7 +418,7 @@
>  @command('email',
>      [('g', 'git', None, _('use git extended diff format')),
>      ('', 'git-format-patch', None, _('use git-format-patch email format '
> -       '(implies --git)')),
> +       '(implies --git and --plain)')),
>      ('', 'plain', None, _('omit hg patch header')),
>      ('o', 'outgoing', None,
>       _('send changes not found in the target repository')),
> @@ -527,6 +527,7 @@
>  
>      if (opts.get('git_format_patch')):
>          opts['git'] = True
> +        opts['plain'] = True
>  
>      if not (opts.get('test') or mbox):
>          # really sending
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -755,13 +755,6 @@
>    To: foo
>    Cc: bar
>    
> -  # HG changeset patch
> -  # User test
> -  # Date 3 0
> -  #      Thu Jan 01 00:00:03 1970 +0000
> -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> -  c

But after reading this, I feel templating will be much nicer.
Henning Schild - Nov. 20, 2016, 11:47 a.m.
On Sun, 20 Nov 2016 19:38:43 +0900
Yuya Nishihara <yuya@tcha.org> wrote:

> On Thu, 17 Nov 2016 21:47:58 +0100, Henning Schild wrote:
> > # HG changeset patch
> > # User Henning Schild <henning@hennsch.de>
> > # Date 1479415557 -3600
> > #      Thu Nov 17 21:45:57 2016 +0100
> > # Node ID de2b03a509491020f728f1955e39e2bfb9a77426
> > # Parent  a9be53e26cb1ac19d1c0156062e8ae23f8366d8b
> > patchbomb: make --git-format-patch imply --plain
> > 
> > Not using --plain would generate mails with the hg header in the
> > git commit message. Since --git-format-patch already caters for
> > git, might as well make sure users do not forget --plain.
> > 
> > diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> > --- a/hgext/patchbomb.py
> > +++ b/hgext/patchbomb.py
> > @@ -418,7 +418,7 @@
> >  @command('email',
> >      [('g', 'git', None, _('use git extended diff format')),
> >      ('', 'git-format-patch', None, _('use git-format-patch email
> > format '
> > -       '(implies --git)')),
> > +       '(implies --git and --plain)')),
> >      ('', 'plain', None, _('omit hg patch header')),
> >      ('o', 'outgoing', None,
> >       _('send changes not found in the target repository')),
> > @@ -527,6 +527,7 @@
> >  
> >      if (opts.get('git_format_patch')):
> >          opts['git'] = True
> > +        opts['plain'] = True
> >  
> >      if not (opts.get('test') or mbox):
> >          # really sending
> > diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> > --- a/tests/test-patchbomb.t
> > +++ b/tests/test-patchbomb.t
> > @@ -755,13 +755,6 @@
> >    To: foo
> >    Cc: bar
> >    
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 3 0
> > -  #      Thu Jan 01 00:00:03 1970 +0000
> > -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> > -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> > -  c  
> 
> But after reading this, I feel templating will be much nicer.

I have played with templates to get "Signed-off-by"s into commit
messages. That works but you have to configure it somewhere. Meaning
that every fresh clone will require dreadful customization, or that
global configuration will impose on every cloned repo.
To me it seems like maintaining my own templates is harder than
maintaining an mq in a custom mercurial. Using them you are basically
programming out of the tree, in a strange and limited language. With a
lot of references of what you expect the hg code to be, but no merge
conflicts on updates, no testing etc. Thinking about it this way,
templating is a fine mechanism for under the hood, but should never
have been exposed to the configs.

Lets assume supporting the git-format-patch format was a wanted
feature, and let us further assume "hg email" already used templates. hg
or hg-git would than come with a template for it, but how would that get
selected?

Henning
Yuya Nishihara - Nov. 21, 2016, 5:17 a.m.
On Sun, 20 Nov 2016 12:47:44 +0100, Henning Schild wrote:
> I have played with templates to get "Signed-off-by"s into commit
> messages. That works but you have to configure it somewhere. Meaning
> that every fresh clone will require dreadful customization, or that
> global configuration will impose on every cloned repo.
> To me it seems like maintaining my own templates is harder than
> maintaining an mq in a custom mercurial. Using them you are basically
> programming out of the tree, in a strange and limited language. With a
> lot of references of what you expect the hg code to be, but no merge
> conflicts on updates, no testing etc. Thinking about it this way,
> templating is a fine mechanism for under the hood, but should never
> have been exposed to the configs.
> 
> Lets assume supporting the git-format-patch format was a wanted
> feature, and let us further assume "hg email" already used templates. hg
> or hg-git would than come with a template for it, but how would that get
> selected?

Maybe "hg email -Tgit" (assuming we have a stock "git" template.)
Henning Schild - Nov. 21, 2016, 7:15 a.m.
Am Mon, 21 Nov 2016 14:17:32 +0900
schrieb Yuya Nishihara <yuya@tcha.org>:

> On Sun, 20 Nov 2016 12:47:44 +0100, Henning Schild wrote:
> > I have played with templates to get "Signed-off-by"s into commit
> > messages. That works but you have to configure it somewhere. Meaning
> > that every fresh clone will require dreadful customization, or that
> > global configuration will impose on every cloned repo.
> > To me it seems like maintaining my own templates is harder than
> > maintaining an mq in a custom mercurial. Using them you are
> > basically programming out of the tree, in a strange and limited
> > language. With a lot of references of what you expect the hg code
> > to be, but no merge conflicts on updates, no testing etc. Thinking
> > about it this way, templating is a fine mechanism for under the
> > hood, but should never have been exposed to the configs.
> > 
> > Lets assume supporting the git-format-patch format was a wanted
> > feature, and let us further assume "hg email" already used
> > templates. hg or hg-git would than come with a template for it, but
> > how would that get selected?  
> 
> Maybe "hg email -Tgit" (assuming we have a stock "git" template.)

If we can agree on that or another command line option, it would just
be a matter of changing the first patch. Actually using templates would
be another series.

Henning
Yuya Nishihara - Nov. 24, 2016, 2:22 p.m.
On Mon, 21 Nov 2016 08:15:44 +0100, Henning Schild wrote:
> Am Mon, 21 Nov 2016 14:17:32 +0900
> schrieb Yuya Nishihara <yuya@tcha.org>:
> > On Sun, 20 Nov 2016 12:47:44 +0100, Henning Schild wrote:
> > > I have played with templates to get "Signed-off-by"s into commit
> > > messages. That works but you have to configure it somewhere. Meaning
> > > that every fresh clone will require dreadful customization, or that
> > > global configuration will impose on every cloned repo.
> > > To me it seems like maintaining my own templates is harder than
> > > maintaining an mq in a custom mercurial. Using them you are
> > > basically programming out of the tree, in a strange and limited
> > > language. With a lot of references of what you expect the hg code
> > > to be, but no merge conflicts on updates, no testing etc. Thinking
> > > about it this way, templating is a fine mechanism for under the
> > > hood, but should never have been exposed to the configs.
> > > 
> > > Lets assume supporting the git-format-patch format was a wanted
> > > feature, and let us further assume "hg email" already used
> > > templates. hg or hg-git would than come with a template for it, but
> > > how would that get selected?  
> > 
> > Maybe "hg email -Tgit" (assuming we have a stock "git" template.)
> 
> If we can agree on that or another command line option, it would just
> be a matter of changing the first patch. Actually using templates would
> be another series.

It sounds worse to me than introducing --git-format-patch option. -T is a
semi-global option, which should work in the same way across commands. (and
I bet no one would be invested to start implementing the full templating
support just to get rid of the temporary -Tgit hack.)

So IMHO, --git-format-patch is better if we need the git-style patch email
without templates. That isn't nice, but we can mark the option as "(DEPRECATED)"
later.
Jun Wu - Nov. 24, 2016, 3:42 p.m.
What do you think about Sean's idea mentioned earlier? Implement the logic
in the hg-git extension. And let the hg-git extension check if the repo is a
hg-git repo or not. We can also have a boolean repo-level config option.

It seems to me that hg-git is a better place (than hg core) to implement all
these features.

Excerpts from Henning Schild's message of 2016-11-17 21:47:58 +0100:
> # HG changeset patch
> # User Henning Schild <henning@hennsch.de>
> # Date 1479415557 -3600
> #      Thu Nov 17 21:45:57 2016 +0100
> # Node ID de2b03a509491020f728f1955e39e2bfb9a77426
> # Parent  a9be53e26cb1ac19d1c0156062e8ae23f8366d8b
> patchbomb: make --git-format-patch imply --plain
> 
> Not using --plain would generate mails with the hg header in the git commit
> message. Since --git-format-patch already caters for git, might as well make
> sure users do not forget --plain.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -418,7 +418,7 @@
>  @command('email',
>      [('g', 'git', None, _('use git extended diff format')),
>      ('', 'git-format-patch', None, _('use git-format-patch email format '
> -       '(implies --git)')),
> +       '(implies --git and --plain)')),
>      ('', 'plain', None, _('omit hg patch header')),
>      ('o', 'outgoing', None,
>       _('send changes not found in the target repository')),
> @@ -527,6 +527,7 @@
>  
>      if (opts.get('git_format_patch')):
>          opts['git'] = True
> +        opts['plain'] = True
>  
>      if not (opts.get('test') or mbox):
>          # really sending
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -755,13 +755,6 @@
>    To: foo
>    Cc: bar
>    
> -  # HG changeset patch
> -  # User test
> -  # Date 3 0
> -  #      Thu Jan 01 00:00:03 1970 +0000
> -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> -  c
>    ---
>     c |  1 +
>     1 files changed, 1 insertions(+), 0 deletions(-)
> @@ -953,13 +946,6 @@
>    To: foo
>    Cc: bar
>    
> -  # HG changeset patch
> -  # User test
> -  # Date 1 0
> -  #      Thu Jan 01 00:00:01 1970 +0000
> -  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
> -  # Parent  0000000000000000000000000000000000000000
> -  a
>    ---
>     a |  1 +
>     1 files changed, 1 insertions(+), 0 deletions(-)
> @@ -989,13 +975,6 @@
>    To: foo
>    Cc: bar
>    
> -  # HG changeset patch
> -  # User test
> -  # Date 2 0
> -  #      Thu Jan 01 00:00:02 1970 +0000
> -  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
> -  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
> -  b
>    ---
>     b |  1 +
>     1 files changed, 1 insertions(+), 0 deletions(-)
Henning Schild - Nov. 30, 2016, 6:30 p.m.
Am Thu, 24 Nov 2016 15:42:58 +0000
schrieb Jun Wu <quark@fb.com>:

> What do you think about Sean's idea mentioned earlier? Implement the
> logic in the hg-git extension. And let the hg-git extension check if
> the repo is a hg-git repo or not. We can also have a boolean
> repo-level config option.

That sounds like a good idea but i am afraid it would be very hard to
hook into "hg email" from hg-git. As far as i understand hg-git, it
deals only with the conversion of changesets or objects. And it does
not really hook into core hg, like hg email.
I guess if email was using templates it would be easy to ship another
template with hg-git and pick it when on git.

Please correct me if i am wrong and suggest a minimal intrusive/hacky
way to implement the changes in hg-git.

> It seems to me that hg-git is a better place (than hg core) to
> implement all these features.

Agreed, and maybe it can be moved out again once templating is
implemented for email.
Aiming at generating mails that look exactly like the ones from git
would require even more changes. So you are right when not wanting to
start with "all these features".

Still i would like to see "git-am" support in hg with hg-git, not
caring too much where it is implemented.

> Excerpts from Henning Schild's message of 2016-11-17 21:47:58 +0100:
> > # HG changeset patch
> > # User Henning Schild <henning@hennsch.de>
> > # Date 1479415557 -3600
> > #      Thu Nov 17 21:45:57 2016 +0100
> > # Node ID de2b03a509491020f728f1955e39e2bfb9a77426
> > # Parent  a9be53e26cb1ac19d1c0156062e8ae23f8366d8b
> > patchbomb: make --git-format-patch imply --plain
> > 
> > Not using --plain would generate mails with the hg header in the
> > git commit message. Since --git-format-patch already caters for
> > git, might as well make sure users do not forget --plain.
> > 
> > diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> > --- a/hgext/patchbomb.py
> > +++ b/hgext/patchbomb.py
> > @@ -418,7 +418,7 @@
> >  @command('email',
> >      [('g', 'git', None, _('use git extended diff format')),
> >      ('', 'git-format-patch', None, _('use git-format-patch email
> > format '
> > -       '(implies --git)')),
> > +       '(implies --git and --plain)')),
> >      ('', 'plain', None, _('omit hg patch header')),
> >      ('o', 'outgoing', None,
> >       _('send changes not found in the target repository')),
> > @@ -527,6 +527,7 @@
> >  
> >      if (opts.get('git_format_patch')):
> >          opts['git'] = True
> > +        opts['plain'] = True
> >  
> >      if not (opts.get('test') or mbox):
> >          # really sending
> > diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> > --- a/tests/test-patchbomb.t
> > +++ b/tests/test-patchbomb.t
> > @@ -755,13 +755,6 @@
> >    To: foo
> >    Cc: bar
> >    
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 3 0
> > -  #      Thu Jan 01 00:00:03 1970 +0000
> > -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> > -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> > -  c
> >    ---
> >     c |  1 +
> >     1 files changed, 1 insertions(+), 0 deletions(-)
> > @@ -953,13 +946,6 @@
> >    To: foo
> >    Cc: bar
> >    
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 1 0
> > -  #      Thu Jan 01 00:00:01 1970 +0000
> > -  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
> > -  # Parent  0000000000000000000000000000000000000000
> > -  a
> >    ---
> >     a |  1 +
> >     1 files changed, 1 insertions(+), 0 deletions(-)
> > @@ -989,13 +975,6 @@
> >    To: foo
> >    Cc: bar
> >    
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 2 0
> > -  #      Thu Jan 01 00:00:02 1970 +0000
> > -  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
> > -  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
> > -  b
> >    ---
> >     b |  1 +
> >     1 files changed, 1 insertions(+), 0 deletions(-)  
>
Sean Farley - Nov. 30, 2016, 7:17 p.m.
Henning Schild <henning@hennsch.de> writes:

> Am Thu, 24 Nov 2016 15:42:58 +0000
> schrieb Jun Wu <quark@fb.com>:
>
>> What do you think about Sean's idea mentioned earlier? Implement the
>> logic in the hg-git extension. And let the hg-git extension check if
>> the repo is a hg-git repo or not. We can also have a boolean
>> repo-level config option.
>
> That sounds like a good idea but i am afraid it would be very hard to
> hook into "hg email" from hg-git. As far as i understand hg-git, it
> deals only with the conversion of changesets or objects. And it does
> not really hook into core hg, like hg email.
> I guess if email was using templates it would be easy to ship another
> template with hg-git and pick it when on git.
>
> Please correct me if i am wrong and suggest a minimal intrusive/hacky
> way to implement the changes in hg-git.

No, hg-git very much digs into the internals of hg. Here is a minimal
example (debugger left in for tutorial purposes) of overriding
`makepatch' for repos with .hg/git:

https://bitbucket.org/seanfarley/hg-git/commits/348c8ff0494d90fe46384eddecbe2676e68ec80f

Hopefully, it's clear that this example modifies patchlines then sends
it to the original (i.e. Mercurial's) `makepatch' method.
Henning Schild - Nov. 30, 2016, 9:23 p.m.
Am Wed, 30 Nov 2016 11:17:02 -0800
schrieb Sean Farley <sean@farley.io>:

> Henning Schild <henning@hennsch.de> writes:
> 
> > Am Thu, 24 Nov 2016 15:42:58 +0000
> > schrieb Jun Wu <quark@fb.com>:
> >  
> >> What do you think about Sean's idea mentioned earlier? Implement
> >> the logic in the hg-git extension. And let the hg-git extension
> >> check if the repo is a hg-git repo or not. We can also have a
> >> boolean repo-level config option.  
> >
> > That sounds like a good idea but i am afraid it would be very hard
> > to hook into "hg email" from hg-git. As far as i understand hg-git,
> > it deals only with the conversion of changesets or objects. And it
> > does not really hook into core hg, like hg email.
> > I guess if email was using templates it would be easy to ship
> > another template with hg-git and pick it when on git.
> >
> > Please correct me if i am wrong and suggest a minimal
> > intrusive/hacky way to implement the changes in hg-git.  
> 
> No, hg-git very much digs into the internals of hg. Here is a minimal
> example (debugger left in for tutorial purposes) of overriding
> `makepatch' for repos with .hg/git:
> 
> https://bitbucket.org/seanfarley/hg-git/commits/348c8ff0494d90fe46384eddecbe2676e68ec80f
> 
> Hopefully, it's clear that this example modifies patchlines then sends
> it to the original (i.e. Mercurial's) `makepatch' method.
> 

Thanks for providing the example code. I will try to come up with a
hg-git-only version.

Henning

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -418,7 +418,7 @@ 
 @command('email',
     [('g', 'git', None, _('use git extended diff format')),
     ('', 'git-format-patch', None, _('use git-format-patch email format '
-       '(implies --git)')),
+       '(implies --git and --plain)')),
     ('', 'plain', None, _('omit hg patch header')),
     ('o', 'outgoing', None,
      _('send changes not found in the target repository')),
@@ -527,6 +527,7 @@ 
 
     if (opts.get('git_format_patch')):
         opts['git'] = True
+        opts['plain'] = True
 
     if not (opts.get('test') or mbox):
         # really sending
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -755,13 +755,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 3 0
-  #      Thu Jan 01 00:00:03 1970 +0000
-  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
-  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
-  c
   ---
    c |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)
@@ -953,13 +946,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 1 0
-  #      Thu Jan 01 00:00:01 1970 +0000
-  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
-  # Parent  0000000000000000000000000000000000000000
-  a
   ---
    a |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)
@@ -989,13 +975,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 2 0
-  #      Thu Jan 01 00:00:02 1970 +0000
-  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
-  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
-  b
   ---
    b |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)