This is an archive of the discontinued Mercurial Phabricator instance.

pull: use opts.get('bookmark') instead of opts['bookmark']
ClosedPublic

Authored by pulkit on Dec 26 2018, 9:44 AM.

Details

Summary

This is done because at places in hgsubversion, we call the function directly. I
expect there might be more instances in extensions out there which calls
commands.push() directly. So let's not require explicitly passing of bookmark
value.

The use of opts['bookmark'] was introduced in
bad05a6afdc89cc58a2af320698ab29bd8de62d4.

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

pulkit created this revision.Dec 26 2018, 9:44 AM
yuja added a subscriber: yuja.Dec 27 2018, 7:12 AM
nodes = None
  • if opts['bookmark'] or revs:

+ if opts.get('bookmark') or revs:

There's one more opts['bookmark']. That's why opts.get('bookmark') was
rewritten as opts[...].

pulkit added inline comments.Dec 27 2018, 7:16 AM
mercurial/commands.py
4451

@yuja this one? I thought if the above condition is true, this will be fine, but now I see or rev.

yuja added a comment.Dec 27 2018, 7:41 AM

commands.py:4441

pullopargs['remotebookmarks'] = remotebookmarks
for b in opts['bookmark']:
    b = repo._bookmarks.expandname(b)

@yuja this one?

Yes.

pulkit updated this revision to Diff 12986.Dec 27 2018, 7:53 AM
yuja added a comment.Dec 27 2018, 8:17 AM

@@ -4438,7 +4438,7 @@

remotebookmarks = fremotebookmarks.result()
remotebookmarks = bookmarks.unhexlifybookmarks(remotebookmarks)
pullopargs['remotebookmarks'] = remotebookmarks
  • for b in opts['bookmark']:

+ for b in opts.get('bookmark'):

Still wrong. It's for b in None.

pulkit updated this revision to Diff 13045.Jan 7 2019, 6:39 AM
This revision was automatically updated to reflect the committed changes.