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 — Infra: Improve email and link processing and rendering in headers by CAM-Gerlach · Pull Request #2467 · python/peps Conversation
Member
After some time reading, writing and interacting with the recently-added automatic formatting of links in the Discussions-To, Post-History and Resolution headers originally added in #2351 and improved in #2361, I've implemented a number of improvements and fixes that make both the URL and the link text more useful, handle many additional common cases, include the Post-History field as well, and prepare for other potential future enhancements. In particular, they now:
This also makes it easy to adopt the Post-History field formatting for the Resolution field, in order to use the much more useful and interesting resolution date as the link text, while retaining the auto-generated venue/target information as the title text, though that will be a separate followup issue/PR. I will have another PR shortly (already almost done) that updates the linters to provide more immediate, specific and precise feedback on the header field format to complement the various recent backend improvements in automatically parsing and rendering them.
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. Looks good, just one nit
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsUh oh!
There was an error while loading. Please reload this page.
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. It would be nice to add unit tests for some of these, especially the text processing functions not doing Sphinx things. Not a blocker for this PR though!
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. For this link I get:
Does this link work for others? Perhaps this later 2016 link withdrawing the PEP would be a good replacement? https://mail.python.org/archives/list/[email protected]/thread/2YMHVPRDWGQLA5A2FKXE2JMLM2HQEEGW/ (As an aside, I note the reason for not renaming .txt to .rst was tooling rather than Git churn, and PEP editors were open to someone doing it)
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. I got the same result as you, and coupled with it breaking the parsing which would require special case workarounds, so I'd removed it, I went ahead and replaced it with the much better link you suggested, both here and in #2484 (which I suggest approving and merging first once it is ready, and dropping the change here).
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
CAM-Gerlach
force-pushed
the
infra-clarify-discussions-to-target-type
branch
from
92994be to
e4f3c01
Compare
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
hugovk
approved these changes
raghavthind2005
approved these changes
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
CAM-Gerlach
force-pushed
the
infra-clarify-discussions-to-target-type
branch
from
e4f3c01 to
7ff9c3a
Compare
CAM-Gerlach
force-pushed
the
infra-clarify-discussions-to-target-type
branch
from
7ff9c3a to
a0bc50f
Compare
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers+1 more reviewerUh oh!
There was an error while loading. Please reload this page. AssigneesLabelsProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. Uh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. | ||||||||||||||||||
| You are visitor 01402520 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |