Patchwork [1,of,6,misc] patchbomb: warn when emailing a dirty working directory parent

login
register
mail settings
Submitter Mads Kiilerich
Date April 7, 2014, 9:11 p.m.
Message ID <ac4eb6bcdb81df536110.1396905093@mk-desktop>
Download mbox | patch
Permalink /patch/4271/
State Accepted
Commit dbff8c119cf6ab0659a9123381d77a2c0bd29947
Headers show

Comments

Mads Kiilerich - April 7, 2014, 9:11 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1396905020 -7200
#      Mon Apr 07 23:10:20 2014 +0200
# Node ID ac4eb6bcdb81df536110ab0b09026539749599a0
# Parent  12f161f08d744f0e4b6eef9c905670afb5c24dd4
patchbomb: warn when emailing a dirty working directory parent
Mads Kiilerich - April 8, 2014, 6:35 p.m.
On 04/08/2014 07:49 PM, David Soria Parra wrote:
> Mads Kiilerich <mads@kiilerich.com> writes:
>
> yes> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1396905020 -7200
>> #      Mon Apr 07 23:10:20 2014 +0200
>> # Node ID ac4eb6bcdb81df536110ab0b09026539749599a0
>> # Parent  12f161f08d744f0e4b6eef9c905670afb5c24dd4
>> patchbomb: warn when emailing a dirty working directory parent
>>
>> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>> --- a/hgext/patchbomb.py
>> +++ b/hgext/patchbomb.py
>> @@ -291,7 +291,11 @@ def patchbomb(ui, repo, *revs, **opts):
>>           return [str(r) for r in revs]
>>   
>>       def getpatches(revs):
>> +        prev = repo['.'].rev()
>>           for r in scmutil.revrange(repo, revs):
>> +            if r == prev and (repo[None].files() or repo[None].deleted()):
>> +                ui.warn(_('warning: working directory has '
>> +                          'uncommitted changes\n'))
> I think the message is slightly missleading. If a user sees this message
> he might assume patchbomb always warns him if his working directory has
> uncommitted changes. However what we really saying is, your are
> patchbombing the current working copy parent and might want to double
> check if you don't accidentally forgot to commit. So we only do this
> in the pctx situation.

Well ...

The message would be long and awkward if we tried to explain everything. 
We show the message when it is relevant - that gives the context in a 
better way than word could describe.

Even if the user should expect that he always got a warning if the 
working directory is dirty (ie no matter if it is relevant or not), I 
don't see how it any way could matter that he in some situation don't 
get the warning.

/Mads
Matt Mackall - April 16, 2014, 5:41 p.m.
On Mon, 2014-04-07 at 23:11 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1396905020 -7200
> #      Mon Apr 07 23:10:20 2014 +0200
> # Node ID ac4eb6bcdb81df536110ab0b09026539749599a0
> # Parent  12f161f08d744f0e4b6eef9c905670afb5c24dd4
> patchbomb: warn when emailing a dirty working directory parent

Queued for default, thanks.

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -291,7 +291,11 @@  def patchbomb(ui, repo, *revs, **opts):
         return [str(r) for r in revs]
 
     def getpatches(revs):
+        prev = repo['.'].rev()
         for r in scmutil.revrange(repo, revs):
+            if r == prev and (repo[None].files() or repo[None].deleted()):
+                ui.warn(_('warning: working directory has '
+                          'uncommitted changes\n'))
             output = cStringIO.StringIO()
             cmdutil.export(repo, [r], fp=output,
                          opts=patch.diffopts(ui, opts))
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -1805,7 +1805,9 @@  no intro message in non-interactive mode
   +b
   
 
-test single flag for single patch:
+test single flag for single patch (and no warning when not mailing dirty rev):
+  $ hg up -qr1
+  $ echo dirt > a
   $ hg email --date '1970-1-1 0:1' -n --flag fooFlag -f quux -t foo -c bar -s test \
   >  -r 2
   this patch series consists of 1 patches.
@@ -1839,9 +1841,10 @@  test single flag for single patch:
   +c
   
 
-test single flag for multiple patches:
+test single flag for multiple patches (and warning when mailing dirty rev):
   $ hg email --date '1970-1-1 0:1' -n --flag fooFlag -f quux -t foo -c bar -s test \
   >  -r 0:1
+  warning: working directory has uncommitted changes
   this patch series consists of 2 patches.
   
   
@@ -1919,6 +1922,8 @@  test single flag for multiple patches:
   @@ -0,0 +1,1 @@
   +b
   
+  $ hg revert --no-b a
+  $ hg up -q
 
 test multiple flags for single patch:
   $ hg email --date '1970-1-1 0:1' -n --flag fooFlag --flag barFlag -f quux -t foo \