This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10437

url_util: introduce a `try_path` function
ClosedPublic

Authored by marmoute on Apr 15 2021, 4:52 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHGc029b35565dd: url_util: introduce a `try_path` function
rHGfd396eb21fea: url_util: introduce a `try_path` function
Summary

That function… try a build a path, returning None on failure. This helps us to simplify various part of the existing code.

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

marmoute created this revision.Apr 15 2021, 4:52 AM
marmoute updated this revision to Diff 26946.Apr 15 2021, 10:39 AM
baymax updated this revision to Diff 27006.Apr 17 2021, 7:39 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare added a subscriber: Alphare.Apr 20 2021, 10:39 AM
Alphare added inline comments.
mercurial/utils/urlutil.py
454

s/are/as/

622

This adds a lookup, why remove the initial KeyError handling?

marmoute added inline comments.Apr 20 2021, 10:51 AM
mercurial/utils/urlutil.py
622

This make the flow clearer, and I don't expect it to be a performance issue. We could also use get, but againt, I don't expect a real difference here.

Alphare accepted this revision.Apr 21 2021, 4:14 AM
Alphare added inline comments.
mercurial/utils/urlutil.py
622

I'm not convinced this is that much clearer, but it's fine.

This revision is now accepted and ready to land.Apr 21 2021, 4:14 AM
baymax updated this revision to Diff 27216.Apr 29 2021, 1:56 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit accepted this revision.Apr 30 2021, 5:50 AM
baymax updated this revision to Diff 27225.Apr 30 2021, 7:58 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute added a commit: rHGfd396eb21fea: url_util: introduce a `try_path` function.Apr 30 2021, 10:08 AM
Closed by commit rHGfd396eb21fea: url_util: introduce a `try_path` function (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/utils/urlutil.py (48 lines)

Diff 27256

mercurial/utils/urlutil.py

def removeauth(u): def removeauth(u):
# type: (bytes) -> bytes # type: (bytes) -> bytes
'''remove all authentication information from a url string''' '''remove all authentication information from a url string'''
u = url(u) u = url(u)
u.user = u.passwd = None u.user = u.passwd = None
return bytes(u) return bytes(u)
def try_path(ui, url):
"""try to build a path from a url
Return None if no Path could built.
"""
try:
# we pass the ui instance are warning might need to be issued
AlphareUnsubmitted
Not Done

s/are/as/

Alphare: s/are/as/
return path(ui, None, rawloc=url)
except ValueError:
return None
def get_push_paths(repo, ui, dests): def get_push_paths(repo, ui, dests):
"""yields all the `path` selected as push destination by `dests`""" """yields all the `path` selected as push destination by `dests`"""
if not dests: if not dests:
if b'default-push' in ui.paths: if b'default-push' in ui.paths:
yield ui.paths[b'default-push'] yield ui.paths[b'default-push']
elif b'default' in ui.paths: elif b'default' in ui.paths:
yield ui.paths[b'default'] yield ui.paths[b'default']
else: else:
"""yields all the `(path, branch)` selected as pull source by `sources`""" """yields all the `(path, branch)` selected as pull source by `sources`"""
if not sources: if not sources:
sources = [b'default'] sources = [b'default']
for source in sources: for source in sources:
if source in ui.paths: if source in ui.paths:
url = ui.paths[source].rawloc url = ui.paths[source].rawloc
else: else:
# Try to resolve as a local path or URI. # Try to resolve as a local path or URI.
try: path = try_path(ui, source)
# we pass the ui instance are warning might need to be issued if path is not None:
url = path(ui, None, rawloc=source).rawloc url = path.rawloc
except ValueError: else:
url = source url = source
yield parseurl(url, default_branches) yield parseurl(url, default_branches)
def get_unique_push_path(action, repo, ui, dest=None): def get_unique_push_path(action, repo, ui, dest=None):
"""return a unique `path` or abort if multiple are found """return a unique `path` or abort if multiple are found
This is useful for command and action that does not support multiple This is useful for command and action that does not support multiple
# XXX this is the historical default behavior, but that is not # XXX this is the historical default behavior, but that is not
# great, consider breaking BC on this. # great, consider breaking BC on this.
url = b'default' url = b'default'
else: else:
if source in ui.paths: if source in ui.paths:
url = ui.paths[source].rawloc url = ui.paths[source].rawloc
else: else:
# Try to resolve as a local path or URI. # Try to resolve as a local path or URI.
try: path = try_path(ui, source)
# we pass the ui instance are warning might need to be issued if path is not None:
url = path(ui, None, rawloc=source).rawloc url = path.rawloc
except ValueError: else:
url = source url = source
return parseurl(url, default_branches) return parseurl(url, default_branches)
def get_clone_path(ui, source, default_branches=()): def get_clone_path(ui, source, default_branches=()):
"""return the `(origsource, path, branch)` selected as clone source""" """return the `(origsource, path, branch)` selected as clone source"""
if source is None: if source is None:
if b'default' in ui.paths: if b'default' in ui.paths:
url = ui.paths[b'default'].rawloc url = ui.paths[b'default'].rawloc
else: else:
# XXX this is the historical default behavior, but that is not # XXX this is the historical default behavior, but that is not
# great, consider breaking BC on this. # great, consider breaking BC on this.
url = b'default' url = b'default'
else: else:
if source in ui.paths: if source in ui.paths:
url = ui.paths[source].rawloc url = ui.paths[source].rawloc
else: else:
# Try to resolve as a local path or URI. # Try to resolve as a local path or URI.
try: path = try_path(ui, source)
# we pass the ui instance are warning might need to be issued if path is not None:
url = path(ui, None, rawloc=source).rawloc url = path.rawloc
except ValueError: else:
url = source url = source
clone_path, branch = parseurl(url, default_branches) clone_path, branch = parseurl(url, default_branches)
return url, clone_path, branch return url, clone_path, branch
def parseurl(path, branches=None): def parseurl(path, branches=None):
'''parse url#branch, returning (url, (branch, branches))''' '''parse url#branch, returning (url, (branch, branches))'''
u = url(path) u = url(path)
except KeyError: except KeyError:
continue continue
return None return None
# Most likely empty string. # Most likely empty string.
# This may need to raise in the future. # This may need to raise in the future.
if not name: if not name:
return None return None
if name in self:
AlphareUnsubmitted
Not Done

This adds a lookup, why remove the initial KeyError handling?

Alphare: This adds a lookup, why remove the initial `KeyError` handling?
marmouteAuthorUnsubmitted
Done

This make the flow clearer, and I don't expect it to be a performance issue. We could also use get, but againt, I don't expect a real difference here.

marmoute: This make the flow clearer, and I don't expect it to be a performance issue. We could also use…
AlphareUnsubmitted
Not Done

I'm not convinced this is that much clearer, but it's fine.

Alphare: I'm not convinced this is that much clearer, but it's fine.
try:
return self[name] return self[name]
except KeyError: else:
# Try to resolve as a local path or URI. # Try to resolve as a local path or URI.
try: path = try_path(ui, name)
# we pass the ui instance are warning might need to be issued if path is None:
return path(ui, None, rawloc=name)
except ValueError:
raise error.RepoError(_(b'repository %s does not exist') % name) raise error.RepoError(_(b'repository %s does not exist') % name)
return path.rawloc
_pathsuboptions = {} _pathsuboptions = {}
def pathsuboption(option, attr): def pathsuboption(option, attr):
"""Decorator used to declare a path sub-option. """Decorator used to declare a path sub-option.