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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 26946

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.