D5969: uncommit: inform user if the commit is empty after uncommit

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -158,6 +158,7 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit files
+  all files uncommitted, the commit is now empty
   $ cat files
   abcde
   foo
@@ -281,6 +282,7 @@
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
+  all files uncommitted, the commit is now empty
   $ hg phase -r .
   15: draft
   $ hg commit --amend -m 'update ab again'
@@ -317,6 +319,7 @@
   > EOS
   $ hg up Q -q
   $ hg uncommit --keep
+  all files uncommitted, the commit is now empty
   $ hg log -G -T '{desc} FILES: {files}'
   @  Q FILES:
   |
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -64,11 +64,11 @@
     if not exclude:
         return None
 
-    files = (initialfiles - exclude)
     # return the p1 so that we don't create an obsmarker later
     if not keepcommit:
         return ctx.p1().node()
 
+    files = (initialfiles - exclude)
     # Filter copies
     copied = copiesmod.pathcopies(base, ctx)
     copied = dict((dst, src) for dst, src in copied.iteritems()
@@ -83,6 +83,9 @@
                                   copied=copied.get(path))
         return mctx
 
+    if not files:
+        repo.ui.status(_("all files uncommitted, the commit is now empty\n"))
+
     new = context.memctx(repo,
                          parents=[base.node(), node.nullid],
                          text=ctx.description(),



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
pulkit added inline comments.

INLINE COMMENTS

> uncommit.py:87
> +    if not files:
> +        repo.ui.status(_("all files uncommitted, the commit is now empty\n"))
> +

how about something like this `(preserving the empty commit)`. The reason is that we need to tell that the empty commit is preserved more than the fact the commit is empty.

note the `(`, it makes it look like a help text.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz updated this revision to Diff 14108.
martinvonz marked an inline comment as done.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5969?vs=14101&id=14108

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -158,6 +158,7 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit files
+  note: keeping empty commit
   $ cat files
   abcde
   foo
@@ -281,6 +282,7 @@
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
+  note: keeping empty commit
   $ hg phase -r .
   15: draft
   $ hg commit --amend -m 'update ab again'
@@ -317,6 +319,7 @@
   > EOS
   $ hg up Q -q
   $ hg uncommit --keep
+  note: keeping empty commit
   $ hg log -G -T '{desc} FILES: {files}'
   @  Q FILES:
   |
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -64,11 +64,11 @@
     if not exclude:
         return None
 
-    files = (initialfiles - exclude)
     # return the p1 so that we don't create an obsmarker later
     if not keepcommit:
         return ctx.p1().node()
 
+    files = (initialfiles - exclude)
     # Filter copies
     copied = copiesmod.pathcopies(base, ctx)
     copied = dict((dst, src) for dst, src in copied.iteritems()
@@ -83,6 +83,9 @@
                                   copied=copied.get(path))
         return mctx
 
+    if not files:
+        repo.ui.status(_("note: keeping empty commit\n"))
+
     new = context.memctx(repo,
                          parents=[base.node(), node.nullid],
                          text=ctx.description(),



To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> pulkit wrote in uncommit.py:87
> how about something like this `(preserving the empty commit)`. The reason is that we need to tell that the empty commit is preserved more than the fact the commit is empty.
>
> note the `(`, it makes it look like a help text.

I added a `note: ` in front instead. I think we usually use parentheses for hints.

I also changed s/preserving/keeping/ to match the term we use in the `--keep` flag (and the upcoming config option).

I kind of like the `all files uncommitted` part since it explains what empty commit we're talking about. Should I add it back?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:87
> I added a `note: ` in front instead. I think we usually use parentheses for hints.
>
> I also changed s/preserving/keeping/ to match the term we use in the `--keep` flag (and the upcoming config option).
>
> I kind of like the `all files uncommitted` part since it explains what empty commit we're talking about. Should I add it back?

> I kind of like the all files uncommitted part since it explains what empty commit we're talking about. Should I add it back?

sure!

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> pulkit wrote in uncommit.py:87
> > I kind of like the all files uncommitted part since it explains what empty commit we're talking about. Should I add it back?
>
> sure!

Hmm, "all files uncommitted, keeping empty commit" makes it sound like the second part follows from the first (which is not correct). I'll just leave it as is.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:87
> Hmm, "all files uncommitted, keeping empty commit" makes it sound like the second part follows from the first (which is not correct). I'll just leave it as is.

And I just realized that we do use parentheses quite frequently for non-hint notes too, such as for "(leaving bookmark test)" and "(no more unresolved files)". Examples of "note: " include "note: commit message saved in .hg/last-message.txt" and "note: graft of 27:17d42b8f5d50 created no changes to commit". It seems there are quite many of each, so it's not clear that one or the other is preferred. I like "note: " better, so I'll leave it as is unless someone else feels strongly.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5969: uncommit: inform user if the commit is empty after uncommit

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG949c4a3b7373: uncommit: inform user if the commit is empty after uncommit (authored by martinvonz, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5969?vs=14108&id=14147

REVISION DETAIL
  https://phab.mercurial-scm.org/D5969

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -159,6 +159,7 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit files
+  note: keeping empty commit
   $ cat files
   abcde
   foo
@@ -282,6 +283,7 @@
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
+  note: keeping empty commit
   $ hg phase -r .
   15: draft
   $ hg commit --amend -m 'update ab again'
@@ -318,6 +320,7 @@
   > EOS
   $ hg up Q -q
   $ hg uncommit --keep
+  note: keeping empty commit
   $ hg log -G -T '{desc} FILES: {files}'
   @  Q FILES:
   |
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -69,11 +69,11 @@
     if not exclude:
         return None
 
-    files = (initialfiles - exclude)
     # return the p1 so that we don't create an obsmarker later
     if not keepcommit:
         return ctx.p1().node()
 
+    files = (initialfiles - exclude)
     # Filter copies
     copied = copiesmod.pathcopies(base, ctx)
     copied = dict((dst, src) for dst, src in copied.iteritems()
@@ -88,6 +88,9 @@
                                   copied=copied.get(path))
         return mctx
 
+    if not files:
+        repo.ui.status(_("note: keeping empty commit\n"))
+
     new = context.memctx(repo,
                          parents=[base.node(), node.nullid],
                          text=ctx.description(),



To: martinvonz, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel