Submitter | phabricator |
---|---|
Date | Jan. 5, 2018, 5:14 p.m. |
Message ID | <differential-rev-PHID-DREV-mhjw77g2tqmtvrxdh74t-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/26569/ |
State | Superseded |
Headers | show |
Comments
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. Needs some warning since a hidden revision may be revived? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1813 To: pulkit, #hg-reviewers, yuja Cc: yuja, mercurial-devel
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > bookmarks.py:856 > if rev: > + warnm = "adding bookmarks to a hidden changeset" > + repo = scmutil.unhidehashlikerevs(repo, [rev], 'warn', warnm) `_("adding ... changeset")` > bookmarks.py:857 > + warnm = "adding bookmarks to a hidden changeset" > + repo = scmutil.unhidehashlikerevs(repo, [rev], 'warn', warnm) > tgt = scmutil.revsingle(repo, rev).node() Accessing hidden revisions here doesn't mean they will be bookmarked. Perhaps we should instead warn if `tgt` is a hidden (or obsolete?) revision. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1813 To: pulkit, #hg-reviewers, yuja Cc: yuja, mercurial-devel
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > bookmarks.py:859 > + if ctx.hidden(): > + repo.ui.warn(_("accessing hidden changeset %s\n") % ctx) > + tgt = ctx.node() Perhaps we can say "bookmarking" or "reviving" instead of "accessing"? I don't know which is better, though. Another minor problem I found is this warning will be repeated if you pass more than one bookmark names. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1813 To: pulkit, #hg-reviewers, yuja Cc: yuja, mercurial-devel
yuja added a comment. Queued, thanks. INLINE COMMENTS > bookmarks.py:853 > + repo.ui.warn(_("bookmarking hidden changeset %s\n") % \ > + (', '.join(hiddenrevs))) > marks.applychanges(repo, tr, changes) FYI, it loops over bookmark names with the same `rev`, so `hiddenrevs` would have at most one element, and the second `unhidehashlikerevs()` call would be unnecessary. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1813 To: pulkit, #hg-reviewers, yuja Cc: yuja, mercurial-devel
Patch
diff --git a/tests/test-directaccess.t b/tests/test-directaccess.t --- a/tests/test-directaccess.t +++ b/tests/test-directaccess.t @@ -186,3 +186,10 @@ abort: hidden revision '2'! (use --hidden to access hidden revisions) [255] + +Setting a bookmark will make that changeset unhidden, so this should come in end + + $ hg bookmarks -r 28ad74 book + + $ hg bookmarks + book 2:28ad74487de9 diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -974,6 +974,9 @@ if not names and (delete or rev): raise error.Abort(_("bookmark name required")) + if rev: + repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn') + if delete or rename or names or inactive: with repo.wlock(), repo.lock(), repo.transaction('bookmark') as tr: if delete: