From 7de9ffc9331f2566e4d658ff3ad06add07acc286 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Tue, 24 Jan 2017 14:58:25 -0500 Subject: [PATCH] support review URLs as download arguments Rather than forcing users to copy only part of a URL to a review, allow them to paste the whole URL and then parse it for them to find the change id. Change-Id: Ic012c86b2b477d17354bf8d119e1d4b698378dd7 Signed-off-by: Doug Hellmann --- git_review/cmd.py | 61 +++++++++++++++++++++++++++-------- git_review/tests/test_unit.py | 60 ++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 13 deletions(-) diff --git a/git_review/cmd.py b/git_review/cmd.py index a456b436..4d82bbe3 100755 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -1377,17 +1377,52 @@ def assert_valid_reviewers(reviewers): "Whitespace not allowed in reviewer: '%s'" % reviewer) +class _DownloadFlag(argparse.Action): + """Special action for the various forms of downloading reviews. + + Additional option parsing: store value in 'dest', but + at the same time set one of the flag options to True + """ + def __call__(self, parser, namespace, value, option_string=None): + url = urlparse(value) + # Turn URLs into change ids: + # https://review.openstack.org/423436 + # and + # https://review.openstack.org/423436/ + # and + # https://review.openstack.org/#/c/423436 + # and + # https://review.openstack.org/c//+/423436 + # become + # "423436" + # while + # https://review.openstack.org/423436/1 + # and + # https://review.openstack.org/#/c/423436/1 + # and + # https://review.openstack.org/c//+/423436/1 + # become + # "423436,1". + # + # If there is a #, the rest of the path is stored in the + # "fragment", otherwise that will be empty. + base = url.fragment or url.path + parts = base.rstrip('/').lstrip('/c').split('/') + # PolyGerrit places the change after a '+' symbol in the url + try: + parts = parts[parts.index('+') + 1:] + except ValueError: + pass + change = parts[0] + if len(parts) > 1: + change = '%s,%s' % (change, parts[1]) + setattr(namespace, self.dest, change) + setattr(namespace, self.const, True) + + def _main(): usage = "git review [OPTIONS] ... [BRANCH]" - class DownloadFlag(argparse.Action): - """Additional option parsing: store value in 'dest', but - at the same time set one of the flag options to True - """ - def __call__(self, parser, namespace, values, option_string=None): - setattr(namespace, self.dest, values) - setattr(namespace, self.const, True) - parser = argparse.ArgumentParser(usage=usage, description=COPYRIGHT) topic_arg_group = parser.add_mutually_exclusive_group() @@ -1438,7 +1473,7 @@ def _main(): fetch.set_defaults(download=False, compare=False, cherrypickcommit=False, cherrypickindicate=False, cherrypickonly=False) fetch.add_argument("-d", "--download", dest="changeidentifier", - action=DownloadFlag, metavar="CHANGE[,PS]", + action=_DownloadFlag, metavar="CHANGE[,PS]", const="download", help="Download the contents of an existing gerrit " "review into a branch. Include the patchset " @@ -1446,26 +1481,26 @@ def _main(): "change. The default is to take the most recent " "version.") fetch.add_argument("-x", "--cherrypick", dest="changeidentifier", - action=DownloadFlag, metavar="CHANGE", + action=_DownloadFlag, metavar="CHANGE", const="cherrypickcommit", help="Apply the contents of an existing gerrit " "review onto the current branch and commit " "(cherry pick; not recommended in most " "situations)") fetch.add_argument("-X", "--cherrypickindicate", dest="changeidentifier", - action=DownloadFlag, metavar="CHANGE", + action=_DownloadFlag, metavar="CHANGE", const="cherrypickindicate", help="Apply the contents of an existing gerrit " "review onto the current branch and commit, " "indicating its origin") fetch.add_argument("-N", "--cherrypickonly", dest="changeidentifier", - action=DownloadFlag, metavar="CHANGE", + action=_DownloadFlag, metavar="CHANGE", const="cherrypickonly", help="Apply the contents of an existing gerrit " "review to the working directory and prepare " "for commit") fetch.add_argument("-m", "--compare", dest="changeidentifier", - action=DownloadFlag, metavar="CHANGE,PS[-NEW_PS]", + action=_DownloadFlag, metavar="CHANGE,PS[-NEW_PS]", const="compare", help="Download specified and latest (or NEW_PS) " "patchsets of an existing gerrit review into " diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py index ef4ad5fa..8104b527 100644 --- a/git_review/tests/test_unit.py +++ b/git_review/tests/test_unit.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import argparse import functools import os import textwrap @@ -352,3 +353,62 @@ class GitReviewUnitTest(testtools.TestCase): check_remote.side_effect = Exception() self.assertRaises(Exception, cmd._main) self.assertTrue(resolve_tracking.called) + + +class DownloadFlagUnitTest(testtools.TestCase): + + def setUp(self): + super(DownloadFlagUnitTest, self).setUp() + self.parser = argparse.ArgumentParser() + self.parser.add_argument( + '-d', + action=cmd._DownloadFlag, + const='download', + dest='cid', + ) + + def test_store_id(self): + args = self.parser.parse_args(['-d', '12345']) + self.assertEqual('12345', args.cid) + + def test_parse_url(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/12345'] + ) + self.assertEqual('12345', args.cid) + + def test_parse_url_trailing_slash(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/12345/'] + ) + self.assertEqual('12345', args.cid) + + def test_parse_url_with_update(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/12345/2'] + ) + self.assertEqual('12345,2', args.cid) + + def test_parse_url_with_hash(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/#/c/12345'] + ) + self.assertEqual('12345', args.cid) + + def test_parse_url_with_hash_and_update(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/#/c/12345/1'] + ) + self.assertEqual('12345,1', args.cid) + + def test_parse_polygerrit_url(self): + args = self.parser.parse_args( + ['-d', + 'https://review.openstack.org/c/org/project/+/12345'] + ) + self.assertEqual('12345', args.cid)