-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Don't let help formatter line-wrap URLs #19825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
They won't be recognized as links without it.
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
You probably should present the diff of --help output between master (rather than 1.17.1) and your commit, in order to ignore other changes that have occurred in the meantime. Anyway, I'm happy to see these changes! I also added the https:// parts in my PR #19073, but in retrospect a more focused PR like this one would have been easier for everyone. |
I'm having trouble "suggesting" this in the GitHub UI, but you might as well delete the obsoleted guidance in the following lines: # 4. The group description should end with a period (unless the last line is a link). If you
# do end the group description with a link, omit the 'http://' prefix. (Some links are too
# long and will break up into multiple lines if we include that prefix, so for consistency
# we omit the prefix on all links.) since we now do the opposite of this. (Furthermore: does your rich link detection work correctly with links that have periods after them? You're a prime candidate to manually test that. If so, just remove all of this.) However, this PR should be merged, in my opinion, whether or not you do that. We can always remove that comment later. |
Ooh, good call, I didn't notice that. (Yeah, suggestions on PR diffs can only suggest changes to the PR, so code that's nowhere near any of my changes (like that comment) isn't eligible. But I'll update that now, thanks for pointing it out!
Good question! At least Gnome Terminal doesn't suck a trailing period into a link. That appears to be special-casing on its part, as RFC3986 §3.5 still seems to be the authoritative standard for URI syntax, and I don't see anything in there (one way or the other) about periods in fragments. So, we probably can't rely on it being safe for all terminal applications. (And, indeed, if I change the long URL in question to https://mypy.readthedocs.io/en/stable/kinds_of_types.html#optional-types-and-the-none-type.fun-type. Gnome Terminal picks up and links everything but the period at the end. So it must be a special case. (Then again, GitHub interprets the bare URL in this comment exactly the same way, so maybe it's pretty widespread after all.) Still, I'm not sure it's a good idea to depend on that. If nothing else, for users of terminals that don't auto-link URLs, double-clicking to select the entire thing for copy-pasting will grab a trailing period if it's there. Probably best to still advise leaving it out. |
Now that URLs in help text are output to the terminal unbroken, so length isn't an issue, we should always include the `https://` scheme identifier to facilitate autodetection. Not following with a period is still good advice, to facilitate manual copy-pasting.
I find many — actually the overwhelming majority of — rich text applications interpret a link with a period at the end "correctly", even though a url could totally have a period at the end according to the url grammar. I'm not sure what the relevant standard for this type of pseudo-url grammar is. But for years now it's ceased to be a practical problem in all applications I personally use. I don't remember any experiences I've had double-clicking to select the entire thing for copy-pasting in a terminal, but probably I've figured manually copy-pasting a link is already something I have to make sure to be exact about, and spent extra focus on getting it right. Maybe mypy formatting should not count on people to have my in-built sense of link paranoia. As a matter of English style in the contemporary era, I suspect it's fine to end a paragraph with a link and no period, anyway. I personally would take out all of that part of the comment — but it's your prerogative, as PR-maker, to do as you see fit with it, in my view. |
So... yeah. I ended up rewriting it to be even longer, instead. 👼 That's purely a symptom of my inclination towards wordiness and overexplaining, tho. I'm happy to cut it down if anyone prefers. |
Oh, and diff updated in the intro comment, as requested. |
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Fixes #19816
This PR adjusts the
AugmentedHelpFormatter
for the command-line--help
output so that it will no longer wrap/break long URLs in the output.It also (in a separate commit) adds
https://
in front of two URLs that lacked it, so they can be recognized as links by anything parsing the help text.In lieu of a test, here's a complete diff between the
--help
output of themaster
branch and this PR (formatting for 80-column width):