Skip to content
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

bpo-35603: Add a note on difflib table header interpreted as HTML #11439

Merged
merged 3 commits into from Sep 11, 2019

Conversation

@tirkarthi
Copy link
Contributor

commented Jan 5, 2019

This adds a doc note on the difflib make_file output. Wording suggestions are welcome.

Output as note :

screen shot 2019-01-06 at 1 11 54 am

Output as warning :

screen shot 2019-01-06 at 1 12 38 am

https://bugs.python.org/issue35603

@epicfaace

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2019

I think the warning looks better. Maybe it would be clearer if you said "fromdesc and todesc are interpreted as unescaped HTML"?

Additionally, it may be better to move the warning right after the paragraph describing fromdesc and todesc.

@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

Additionally, it may be better to move the warning right after the paragraph describing fromdesc and todesc.

Thanks for the feedback. From what I have seen in docs notes/warnings are generally grouped and placed at the end of docs for a function/class and I feel it can distract the user from rest of docs for make_file if it's in the middle . I have used notes because I don't know if this is intentional or missed during the initial API which was developed during python 2.7.

@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

I think the warning looks better. Maybe it would be clearer if you said "fromdesc and todesc are interpreted as unescaped HTML"?

Rephrased note as suggested. Thanks!

@serhiy-storchaka
Copy link
Member

left a comment

I think it is better to place the note above the versionchanged directive.

@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

@serhiy-storchaka I have placed it above versionchanged . I just checked the docs and there doesn't seem to be any convention over this. Local build screenshot as below :

screen shot 2019-01-08 at 5 36 43 pm

@csabella csabella requested a review from serhiy-storchaka Jun 5, 2019

@JulienPalard JulienPalard merged commit c78dae8 into python:master Sep 11, 2019

5 checks passed

Azure Pipelines PR #20190108.12 succeeded
Details
bedevere/issue-number Issue number 35603 found
Details
bedevere/news "skip news" label found
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Copy link

commented Sep 11, 2019

Thanks @tirkarthi for the PR, and @JulienPalard for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒🤖 I'm not a witch! I'm not a witch!

miss-islington added a commit to miss-islington/cpython that referenced this pull request Sep 11, 2019
bpo-35603: Add a note on difflib table header interpreted as HTML (py…
…thonGH-11439)

(cherry picked from commit c78dae8)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
@bedevere-bot

This comment has been minimized.

Copy link

commented Sep 11, 2019

GH-15922 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Sep 11, 2019
bpo-35603: Add a note on difflib table header interpreted as HTML (GH…
…-11439)

(cherry picked from commit c78dae8)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.