Skip to content

Handle pep files with .rst extensions. #32

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

Merged
merged 2 commits into from
Jun 27, 2016
Merged

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented Jun 23, 2016

Not completely sure how that would interact with deploying pep on pythondotorg and other services.

@@ -469,7 +471,7 @@ def pep_type_error(inpath, pep_type):
def browse_file(pep):
import webbrowser
file = find_pep(pep)
if file.endswith(".txt"):
if (file.startswith('pep-') and (file.endswith(".txt") or file.endswith('.rst'))):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file.endswith((".txt", ".rst")) would be clearer.

@berkerpeksag
Copy link
Member

Not completely sure how that would interact with deploying pep on pythondotorg and other services.

I don't have time to test it this week, but it looks like a backward compatible change to me.

Thanks!

@Carreau
Copy link
Contributor Author

Carreau commented Jun 24, 2016

The only undefined behavior is if txt and rst are present.

I'll update with your suggestions later.
On Jun 23, 2016 16:16, "Berker Peksag" [email protected] wrote:

Not completely sure how that would interact with deploying pep on
pythondotorg and other services.

I don't have time to test it this week, but it looks like a backward
compatible change to me.

Thanks!


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#32 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAUez3cBSWYX5aFDmAkD6FZ1HIfuEtWYks5qOxOygaJpZM4I9UeF
.

@Carreau
Copy link
Contributor Author

Carreau commented Jun 25, 2016

Updated as per comment.

@Carreau
Copy link
Contributor Author

Carreau commented Jun 25, 2016

You can see the difference in the generated html here , which is mostly due to the random.choice of banner, and (strangely) a couple of author that are not sorted in the same order in pep0. (investigating, but might be unrelated to that).

@@ -41,7 +41,7 @@ def main(argv):
abs_file_path = os.path.join(path, file_path)
if not os.path.isfile(abs_file_path):
continue
if file_path.startswith("pep-") and file_path.endswith(".txt"):
if file_path.startswith("pep-") and file_path[-4:] in (".txt", "rst"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_path.endswith((".txt", ".rst")) would work too.

@ncoghlan
Copy link
Contributor

Big +1 for the general idea, since setting the file extension will let GitHub render them nicely.

At the detailed level, I didn't have anything to add to Berker's existing comments.

@ncoghlan
Copy link
Contributor

That said, would it make sense to update https://www.python.org/dev/peps/pep-0012/ at the same time as making the changes to support the .rst extension?

It turns out that needs an update anyway to refer to the GitHub repo rather than hg.python.org

@Carreau
Copy link
Contributor Author

Carreau commented Jun 25, 2016

Will do both early next week.
On Jun 25, 2016 13:52, "Nick Coghlan" [email protected] wrote:

That said, would it make sense to update
https://www.python.org/dev/peps/pep-0012/ at the same time as making the
changes to support the .rst extension?

It turns out that needs an update anyway to refer to the GitHub repo
rather than hg.python.org


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#32 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAUezy0XtUpRXCeW2-TqTjsaA0UZc7yGks5qPZT8gaJpZM4I9UeF
.

@Carreau
Copy link
Contributor Author

Carreau commented Jun 27, 2016

Updated as well as pep12.

@brettcannon brettcannon merged commit c7adf11 into python:master Jun 27, 2016
@brettcannon
Copy link
Member

Thanks for this, @Carreau !

@Carreau Carreau deleted the rst-peps branch June 27, 2016 18:09
lukpueh pushed a commit to lukpueh/peps that referenced this pull request Oct 17, 2019
Clarify that we are recommending Ed25519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants