Details
- Reviewers
- None
- Group Reviewers
hg-reviewers
Diff Detail
- Repository
- rHG Mercurial
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
Didn't looked at the code in detail yet, but this patch needs tests.
Also, @yuja how do you think of the following:
0. I move metaedit command to core
- I move branch changing functionality to metaedit
- and then we implement hg branch -r <rev> to show a branch name?
fm.startitem()+ rev = ctx.rev()
+ if opts.get('rev') and rev not in revs:
+ continue
ctx points to the tipmost branch head. so rev not in revs doesn't mean any
of the revs do not belong to the branch.
0. I move metaedit command to core 1. I move branch changing functionality to metaedit
No opinion about 0 and 1. I've never used the metaedit.
- and then we implement hg branch -r <rev> to show a branch name?
I want to avoid it because no other namespace commands (i.e. bookmark and tag)
behave as such.
Is there any possibility of workaround within this iteration itself @yuja? I can do outside the current loop by creating a branches list and map it by iterating over scmutil.revrange() as suggested by you. But, that won't fetch all details. Just changeset and corresponding branch. We need to keep the same output format, right?
> ctx points to the tipmost branch head. so `rev not in revs` doesn't mean any > of the `revs` do not belong to the branch. Is there any possibility of workaround within this iteration itself @yuja? I can do outside the current loop by creating a branches list and map it by iterating over scmutil.revrange() as suggested by you. But, that won't fetch all details. Just changeset and corresponding branch. We need to keep the same output format, right?
Well, I meant you can build a set of branches to select.
selectedbranches = {repo[r].branch() for r in revs} for tag, ... in repo.branchmap().iterbranches(): if tag not in selectedbranches: continue ...
This is the simplest implementation. Since repo[r].branch() isn't a cheap
operation, it's probably better to query a revbranchcache.
getbi = repo.revbranchcache().branchinfo selectedbranches = {getbi(r)[0] for r in revs}
There will be more optimization ideas (e.g. look up branchmap directly if
len(revs) == 1, but they won't be required to implement a minimal working
patch.
@yuja Thank you so much for the information. I've updated the revision. Please review when you're free.
Looks mostly good.
Can you update the commit message to conform to our style?
https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist
You'll see some lint errors if you run test-check-*. And you'll probably
need to update test-completion.t.
opts = pycompat.byteskwargs(opts)+ revs = opts.get('rev')
+ if revs:
+ revs = scmutil.revrange(repo, revs)
+ getbi = repo.revbranchcache().branchinfo
+ selectedbranches = {getbi(r)[0] for r in revs}
+ui.pager('branches') fm = ui.formatter('branches', opts) hexfunc = fm.hexfunc@@ -1165,6 +1172,8 @@
allheads = set(repo.heads()) branches = [] for tag, heads, tip, isclosed in repo.branchmap().iterbranches():+ if revs and tag not in selectedbranches:
+ continue
I prefer initializing selectedbranches to None, and check if it is None
here. It makes sure that selectedbranches never be an undefined name, and
avoid weird behavior when revs resolved to an empty set.
Updated.
You'll see some lint errors if you run test-check-*. And you'll probably
need to update test-completion.t.
I've updated test-completion.t. But, I didn't see any lint errors on running test-check-*.
opts = pycompat.byteskwargs(opts)+ revs = opts.get('rev')
+ if revs:
+ revs = scmutil.revrange(repo, revs)
+ getbi = repo.revbranchcache().branchinfo
+ selectedbranches = {getbi(r)[0] for r in revs}
+ui.pager('branches') fm = ui.formatter('branches', opts) hexfunc = fm.hexfunc@@ -1165,6 +1172,8 @@
allheads = set(repo.heads()) branches = [] for tag, heads, tip, isclosed in repo.branchmap().iterbranches():+ if revs and tag not in selectedbranches:
+ continueI prefer initializing selectedbranches to None, and check if it is None
here. It makes sure that selectedbranches never be an undefined name, and
avoid weird behavior when revs resolved to an empty set.
I didn't think of this. Thanks for pointing that out. It's fixed in this patch.
I tried to queue this, but the patch doesn't include any metadata (e.g. author
and date.) Please check your configuration.
https://www.mercurial-scm.org/wiki/Phabricator
+ revs = opts.get('rev')
+ selectedbranches = None
+ if revs:
+ revs = scmutil.revrange(repo, revs)
+ getbi = repo.revbranchcache().branchinfo
+ selectedbranches = {getbi(r)[0] for r in revs}
+ui.pager('branches') fm = ui.formatter('branches', opts) hexfunc = fm.hexfunc@@ -1165,6 +1173,8 @@
allheads = set(repo.heads()) branches = [] for tag, heads, tip, isclosed in repo.branchmap().iterbranches():+ if selectedbranches and tag not in selectedbranches:
Please update this to selectedbranches is not None. You can see a difference
with -r 'none()'.
We like the UX of hg branch --rev R XXX for changing the branch of a commit, it's the same UX than bookmark and topic. It's also easier than hg branch --force XXX; hg amend.
We like the UX of `hg branch --rev R XXX` for changing the branch of a commit, it's the same UX than bookmark and topic. It's also easier than `hg branch --force XXX; hg amend`.
What I don't like about hg branch --rev is that it actually rewrites
the commits, which is IMHO different in UX from hg branch and hg bookmark.
It's also different in that both hg branch (with no rev) and hg bookmark [-r]
specify a branch head (suppose the working change will eventually be new
branch head), whereas hg branch -r REV specifies a set of revisions to be
re-branched.
(Maybe we should move the discussion to the list if it continues. I hate Phab
emails.)