This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5969

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

Authored by martinvonz on Feb 15 2019, 2:12 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG83d294c71f1e: uncommit: inform user if the commit is empty after uncommit
rHGc10652d1328e: uncommit: inform user if the commit is empty after uncommit
rHG949c4a3b7373: uncommit: inform user if the commit is empty after uncommit

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Feb 15 2019, 2:12 PM
pulkit added a subscriber: pulkit.Feb 15 2019, 2:59 PM
pulkit added inline comments.
hgext/uncommit.py
92

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.

martinvonz marked an inline comment as done.Feb 15 2019, 3:19 PM
martinvonz updated this revision to Diff 14108.
martinvonz added inline comments.Feb 15 2019, 3:20 PM
hgext/uncommit.py
92

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?

pulkit added inline comments.Feb 15 2019, 3:27 PM
hgext/uncommit.py
92

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!

martinvonz added inline comments.Feb 15 2019, 3:31 PM
hgext/uncommit.py
92

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.

martinvonz added inline comments.Feb 15 2019, 4:17 PM
hgext/uncommit.py
92

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.

pulkit accepted this revision.Feb 19 2019, 9:53 AM
Closed by commit rHG949c4a3b7373: uncommit: inform user if the commit is empty after uncommit (authored by martinvonz). · Explain WhyFeb 20 2019, 3:16 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/uncommit.py (5 lines)
Mtests/test-uncommit.t (3 lines)

Diff 14147

hgext/uncommit.py

# ctx # ctx
initialfiles = set(ctx.files()) initialfiles = set(ctx.files())
exclude = set(f for f in initialfiles if match(f)) exclude = set(f for f in initialfiles if match(f))
# No files matched commit, so nothing excluded # No files matched commit, so nothing excluded
if not exclude: if not exclude:
return None return None
files = (initialfiles - exclude)
# return the p1 so that we don't create an obsmarker later # return the p1 so that we don't create an obsmarker later
if not keepcommit: if not keepcommit:
return ctx.p1().node() return ctx.p1().node()
files = (initialfiles - exclude)
# Filter copies # Filter copies
copied = copiesmod.pathcopies(base, ctx) copied = copiesmod.pathcopies(base, ctx)
copied = dict((dst, src) for dst, src in copied.iteritems() copied = dict((dst, src) for dst, src in copied.iteritems()
if dst in files) if dst in files)
def filectxfn(repo, memctx, path, contentctx=ctx, redirect=()): def filectxfn(repo, memctx, path, contentctx=ctx, redirect=()):
if path not in contentctx: if path not in contentctx:
return None return None
fctx = contentctx[path] fctx = contentctx[path]
mctx = context.memfilectx(repo, memctx, fctx.path(), fctx.data(), mctx = context.memfilectx(repo, memctx, fctx.path(), fctx.data(),
fctx.islink(), fctx.islink(),
fctx.isexec(), fctx.isexec(),
copied=copied.get(path)) copied=copied.get(path))
return mctx return mctx
if not files:
repo.ui.status(_("note: keeping empty commit\n"))
pulkitUnsubmitted
Done

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.

pulkit: how about something like this `(preserving the empty commit)`. The reason is that we need to…
martinvonzAuthorUnsubmitted
Not Done

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?

martinvonz: I added a `note: ` in front instead. I think we usually use parentheses for hints. I also…
pulkitUnsubmitted
Not Done

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!

pulkit: > I kind of like the all files uncommitted part since it explains what empty commit we're…
martinvonzAuthorUnsubmitted
Not Done

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.

martinvonz: Hmm, "all files uncommitted, keeping empty commit" makes it sound like the second part follows…
martinvonzAuthorUnsubmitted
Not Done

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.

martinvonz: And I just realized that we do use parentheses quite frequently for non-hint notes too, such as…
new = context.memctx(repo, new = context.memctx(repo,
parents=[base.node(), node.nullid], parents=[base.node(), node.nullid],
text=ctx.description(), text=ctx.description(),
files=files, files=files,
filectxfn=filectxfn, filectxfn=filectxfn,
user=ctx.user(), user=ctx.user(),
date=ctx.date(), date=ctx.date(),
extra=ctx.extra()) extra=ctx.extra())

tests/test-uncommit.t

abcde abcde
foo foo
$ hg status $ hg status
M files M files
$ hg uncommit $ hg uncommit
abort: uncommitted changes abort: uncommitted changes
[255] [255]
$ hg uncommit files $ hg uncommit files
note: keeping empty commit
$ cat files $ cat files
abcde abcde
foo foo
$ hg commit --amend -m "files abcde + foo" $ hg commit --amend -m "files abcde + foo"
Testing the 'experimental.uncommitondirtywdir' config Testing the 'experimental.uncommitondirtywdir' config
$ echo "bar" >> files $ echo "bar" >> files
$ hg uncommit $ hg uncommit
$ hg phase -r . $ hg phase -r .
12: draft 12: draft
$ hg commit -m 'update ab again' $ hg commit -m 'update ab again'
Phase is preserved Phase is preserved
$ hg uncommit --keep --config phases.new-commit=secret $ hg uncommit --keep --config phases.new-commit=secret
note: keeping empty commit
$ hg phase -r . $ hg phase -r .
15: draft 15: draft
$ hg commit --amend -m 'update ab again' $ hg commit --amend -m 'update ab again'
Uncommit with public parent Uncommit with public parent
$ hg phase -p "::.^" $ hg phase -p "::.^"
$ hg uncommit $ hg uncommit
$ cd repo1 $ cd repo1
$ hg debugdrawdag <<'EOS' $ hg debugdrawdag <<'EOS'
> Q > Q
> | > |
> P > P
> EOS > EOS
$ hg up Q -q $ hg up Q -q
$ hg uncommit --keep $ hg uncommit --keep
note: keeping empty commit
$ hg log -G -T '{desc} FILES: {files}' $ hg log -G -T '{desc} FILES: {files}'
@ Q FILES: @ Q FILES:
| |
| x Q FILES: Q | x Q FILES: Q
|/ |/
o P FILES: P o P FILES: P
$ hg status $ hg status