GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — PEP 12: Recommend and explain native RST hyperlinks (versus footnotes) by CAM-Gerlach · Pull Request #2302 · python/peps Conversation
Member
As a followup to #2155 , which stops auto-rendering link targets as footnotes, and hides the References section if only link targets are used (as well as #2227 and #2229 which fixed bugs with its initial implementation), and #2291, which made initial changes to PEP 12 to recommend the new PEP and RFC roles instead of footnotes for referencing PEP, this PR finally finishes implementing #2130 by further updating PEP 12 to recommend using native RST hyperlinks over manual footnotes for referencing URLs, as well as better explaining how to do so. This PR also changes the suggested title from "References" to "Footnotes" to reflect its use and contents, updates a couple lines in PEP 1 and the template that reference the old use, and tweaks a few links in PEP 0, PEP 1, PEP 12 to follow the new recommendations, as these PEPs are highly-active meta-PEPs and exemplars and templates for the others. While there seemed to be general consensus among the PEP editors on my suggested course of action on #2130, including the revised suggested section title to be more appropriate to its contents (as already used on some PEPs), I specifically made that as a separate commit, so if for whatever reason, we don't want to change it for new PEPs, I should be able to (more or less) cleanly drop that commit. (Finally) closes #2130
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
CAM-Gerlach
requested review from
AA-Turner,
brettcannon,
ncoghlan and
warsaw
as code owners
CAM-Gerlach
marked this pull request as draft
CAM-Gerlach
force-pushed
the
pep-12-update-references-language
branch
from
da2ec83 to
42e7d3b
Compare
CAM-Gerlach
marked this pull request as ready for review
hugovk
reviewed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A question or two :)
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this line also be part of the RST code block so it's visible as an example?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, but actually, I do include it, in the appropriate example where I talk about link targets, which are discussed separately from link sources.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this line needed? It's not used in this example.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, good catch—this is a copy/paste error and was already included above. Oops!
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question: should this line also be part of the RST code block so it's visible as an example?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like the other one, it is listed in the RST code block immediately below that discusses link targets.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
brettcannon
reviewed
Comment on lines
+457
to
+462
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth giving so much detail on how to write a link or to link to the reST docs at https://docutils.sourceforge.io/docs/user/rst/quickref.html#indirect-hyperlink-targets and provide the examples? Same below for the other link styles.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree -- I'd suggest most of this section could be replaced with a pointer to the Docutils reference, perhaps excepting not to use raw URLs and to prefer HTTPS. I'd go so far as to remove the examples, too.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
| 12. Footnotes -- A collection of footnotes referenced in the PEP, and | ||
| a central place to list non-inline hyperlink targets. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Wordsmithing -- should we avoid "referenced" to avoid confusion with inline references? Perhaps "used"?
"a central place to list non-inline hyperlink targets" -- do we want to still reccoment this? 682 which I just reviewed put links at the end of each section, which was actually quite nice in terms of reading the raw reST source. It doesn't matter from a rendering perspective but might make review easier?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Wordsmithing -- should we avoid "referenced" to avoid confusion with inline references? Perhaps "used"?
Good point; though "used" isn't really accurate either, since the footnotes themselves aren't actually "used" in the PEP, but rather linked. "cited" is a good substitute, though, which I've replaced it with here as well as the template.
"a central place to list non-inline hyperlink targets" -- do we want to still reccoment [sic] this?
I wouldn't go so far as to say the current language "recommends" (assuming that's the word you mean) this approach, as opposed to simply offers it as an option; I removed "central" both places to put a little less empahsis on this.
I do recommend the "links after each paragraph" approach (that's much more proximate and predictable than after each section) for documentation in the Spyder Documentation Style Guide, but I don't think it makes sense to not at least offer a centralized list as an option, as most current PEPs use it, and it is very useful for PEPs that have large numbers of links or have many links with multiple uses (both of which made it invaluable for PEP 639), to list them in one common, known location.
So I'd favor not being overly prescriptive and breaking with current practice here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| The source for this (or any) PEP can be found in the PEPs repository, | ||
| viewable on the web at https://github.com/python/peps/ . | ||
| The source for this (or any) PEP can be found in the | ||
| `PEPs repository <https://github.com/python/peps/>`_, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Can you also link the one on line 15 please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I guess so, though it is a bit out of scope here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| - Update your References and Copyright section. Usually you'll place | ||
| your PEP into the public domain, in which case just leave the | ||
| Copyright section alone. Alternatively, you can use the `Open | ||
| Publication License`__, but public domain is still strongly | ||
| preferred. | ||
|
|
||
| __ http://www.opencontent.org/openpub/ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I only skimmed #2130, but I didn't see a copyright discussion -- there are a few PEPs licenced under the OPL (437, 483, 551, 578, 3145). For some authors there may be issues with PD/CC0, so noting the option exists may be useful?
Personally I am fine with PD (having released the code in this repo to it) -- and even better if I missed the dicussion & all good, but better safe than sorry & all that!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
An option doesn't exist; all PEPs must be dual-licensed CC0 and "PD" (which is effectively the same as just licensing CC0, which in turn is a public domain declaration) per PEP 1 as implemented in #1117 per the discussion with PSF legal in #123 ; the lack of a corresponding update to PEP 12 was presumably an oversight. The OPL (which itself is an ambiguous acronym, as it could refer to either the Open Content License or the Open Publication License, two different licenses) is an obsolete, ambiguous, incompatible and likely legally flawed license; it would be strongly preferable to get those old PEPs relicensed, but that is entirely out of scope here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ahh, thanks for the links -- important that this gets updated, then.
A
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| If you intend to only reference a link once, and want to define it inline | ||
| with the text, insert the link into angle brackets (``<>``) after the text | ||
| you want to link, but before the closing backtick, with a space between the | ||
| text and the opening backtick. You should also use a double-underscore after | ||
| the closing backtick instead of a single one, which makes it an anonymous | ||
| reference to avoid conflicting with other target names. For example: |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I agree -- I'd suggest most of this section could be replaced with a pointer to the Docutils reference, perhaps excepting not to use raw URLs and to prefer HTTPS.
I'd go so far as to remove the examples, too.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| Footnotes | ||
| ========= | ||
|
|
||
| .. [1] Note that the footnote reference is a numbered one. | ||
|
|
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove this bit and only recommend [#cite] style footnotes, numbered is always a pain to update and leaves a large diff.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Where "this bit" is the section that explains numbered footnotes
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I agree; I just didn't make that change out of conservatism, but I went ahead with it now.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if "references" in title_words or "footnotes" in title_words: | ||
| # Remove references/footnotes section if there are no displayed |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ideally I'd use a set literal on the line above ({*section[0].astext().lower().split()} instead of {*title_words}) but GH won't let me put a suggestion on that line and also this line for reasons man has yet to understand.
| if "references" in title_words or "footnotes" in title_words: | |
| # Remove references/footnotes section if there are no displayed | |
| if {"references", "footnotes"} & {*title_words}: | |
| # Remove the references or footnotes sections if there are no displayed |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point—I've manually made the change both here and in pep2html.py.
GH won't let me put a suggestion on that line and also this line for reasons man has yet to understand.
I may or may not be a "man", but the reason, dumb as it is, is that intervening deleted lines that it doesn't like.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I had thought I'd left replies to most of the comments here a week ago, but it turns out they never got posted while I was on my trip down the SpaceX production and launch site in Texas that I got invited to. I'll go through them again and hopefully reply for real this time, now that I have some. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
fa28adb to
78e97c7
Compare
78e97c7 to
be51524
Compare
|
This has been discussed a lot and everyone seems to be in agreement that this is generally a good idea, so I'll merge this now. Slimming down the reST primer and clarifying the role of PEP 1 and PEP 12 can be done in later PRs (I might have a go myself, time dependent). A |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
As a note, it would probably be wise for us to gain consensus first on the issue, and to decide if and how we're going to break down and share the task, to avoid duplicate/conflicted effort and wasting our always-valuable limited volunteer bandwidth. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Discourage using References?
There was an error while loading. Please reload this page.