bpo-29976: urllib.parse clarify '' in scheme values. #984
Conversation
|
@orsenthil, thanks for your PR! By analyzing the history of the files in this pull request, we identified @serhiy-storchaka, @ncoghlan and @Yhg1s to be potential reviewers. |
|
Add 'bpo-29976: ' to the beginning of the title to invoke auto-linking of PR and status back to the issue. |
orsenthil
changed the title
|
@terryjreedy - thanks for the note, I missed that one! |
Lib/urllib/parse.py
Outdated
|
|
||
| RFC 3986 is considered the current standard and any future changes to | ||
| urlparse module should conform with it. The urlparse module is | ||
| currently not entirely compliant with this RFC due to defacto | ||
| currently not entirely compliant with this RFC due to de-facto |
There was a problem hiding this comment.
'de facto', without '-'. See for instance https://www.merriam-webster.com/dictionary/defacto
Lib/urllib/parse.py
Outdated
| uses_relative = ['ftp', 'http', 'gopher', 'nntp', 'imap', | ||
| # A classification of schemes. | ||
| # We have '' as scheme value to accommodate for the default value of scheme | ||
| # arg in urlsplit and urlparse. |
There was a problem hiding this comment.
Better, but without having used argparse yet, I think something like "Use ' ' as the scheme value to get the default scheme in urlsplit and urlparse." might be even better.
There was a problem hiding this comment.
I settled for this. # We use '' as the scheme value for default scheme in urlsplit and urlparse.
Lib/urllib/parse.py
Outdated
| # A classification of schemes. | ||
| # We use '' as the scheme value for default scheme in urlsplit and urlparse. | ||
|
|
||
| uses_relative = ['', 'ftp', 'http', 'gopher', 'nntp', 'imap', |
There was a problem hiding this comment.
Uses_relative only seems to affect “urljoin”, not “urlsplit” nor “urlparse”. The empty string means that you can join one relative URL onto another one.
There was a problem hiding this comment.
The comment applies to all three lists, uses_relative, uses_netloc and uses_params.
All of them have '' (empty string) listed as supported scheme, because that is the default
def urlsplit(url, scheme='', allow_fragments=True):
and
def urlparse(url, scheme='', allow_fragments=True):
And the current comment is:
# A classification of schemes ('' means apply by default)
- I think, the existing comment is confusing, and thus, this change is to make it a bit clear.
There was a problem hiding this comment.
Ah, I misunderstood the comment. I thought you were listing the functions that the lists affected. What about something like
The empty string classifies relative URLs with no scheme specified, being the default returned by “urlsplit” and “urlparse”
There was a problem hiding this comment.
Sounds good to me. I will write it as "default value".
The empty string classifies relative URLs with no scheme specified, being the default value returned by “urlsplit” and “urlparse”
There was a problem hiding this comment.
Modified the comment as per our discussion here.
|
IMO the changes to |
|
Removed the unrelated docstring changes and I guess, this is good to go. |
Issue for discussion: http://bugs.python.org/issue29976