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 <doug@doughellmann.com>
This commit is contained in:
Doug Hellmann 2017-01-24 14:58:25 -05:00 committed by Darragh Bailey
parent ed3c79e452
commit 7de9ffc933
2 changed files with 108 additions and 13 deletions

View File

@ -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/<project>/+/423436
# become
# "423436"
# while
# https://review.openstack.org/423436/1
# and
# https://review.openstack.org/#/c/423436/1
# and
# https://review.openstack.org/c/<project>/+/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 "

View File

@ -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)