-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add Markdown support #3
Conversation
d40faf9
to
fce3394
Compare
It would be nice to change the tests to use .md and .html files in I'll try to find some time to do that. |
Most projects using Github Flavored Markdown. |
@methane: Good point about GitHub Markdown. I'll have to add tests for that and then try out the extensions you mentioned. Feel free to throw it in there if you get a chance. |
By the way, how PyPI can determine that which format long_description written?
|
@methane: Good point. You might be interested in the discussion around this at https://bitbucket.org/pypa/pypi/issue/148/support-markdown-for-readmes |
@dstufft: If you have comments, I can update this. |
I don't think it makes sense to merge any Markdown support until we have an answer for how PyPI (or any other tool) is supposed to know what format it's supposed to use. |
|
||
def read(fn): | ||
path = os.path.join(os.path.dirname(__file__), 'fixtures', 'markdown', fn) | ||
with codecs.open(path, encoding='utf-8') as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use io.open
instead. codecs.open
will be deprecated: http://bugs.python.org/issue8796
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks!
I don't think we should block this merge on "how PyPI (or any other tool) is supposed to know what format it's supposed to use"; we can merge this without regressing on that front, and there are a few obvious ways to improve it later. |
0150140
to
75e55a5
Compare
@bukzor: I agree. I think that the rendering of Markdown vs. the decision of what markup language to render are orthogonal issues. It's possible that pypa/README could attempt to guess the format from a blob of text using some heuristics, but I think that the more robust solution is to have something at a higher level that guesses the format based on filename extension or even better, a new piece of distribution metadata -- e.g.: I don't think figuring out the mechanism for choosing the renderer needs to block making progress on making Markdown rendering work nicely (which I think largely means having parity with GitHub-flavored Markdown). |
What's the point of being able to render Markdown without being able to determine if we should render as Markdown? Without some way to determine what we should use this code is just going to sit there doing nothing. |
Well yeah, I get that the Markdown stuff won't be used until we figure out how. But I think we should try to work on both. One could make the reverse argument -- What's the point of having a mechanism for identifying different content types and choosing between them when there is only one markup format? I don't like having dead code but my bigger concern would be that if we couple two efforts together then each one stalls the other and increases the chances of a deadlock. |
We already have two rendering formats, plaintext and rst. The selection mechanism is implicit and fallback based. This won't work if we add in a third rendering format but anything that works for 3 formats will work for 2 formats. A better mechanism to choose between them would mean that PyPI could reject uploads which specify rst and which fail at rst rendering (for instance). There's no dependency on Markdown for being able to be explicit about how you want the long description rendered. There is a dependency on being explicit about how to render the long description before we can actually render anything with Markdown. |
@dstufft: You make some valid points, but I don't see it as an argument to block this merge, let this work bitrot. As @msabramo noted, once this is merged, everyone will be much happier to work on a content-type switching mechanism. It may not be the order your prefer, but it's certainly the order it's been worked on. |
Well put, @bukzor! I also think @dstufft made some valid points about how the content type switching could be useful without Markdown. Which just confirms for me that these two things are independent of each other and could happen in either order. So why not just tackle them in the order that they naturally unfold so that we maintain forward momentum? |
Content type switching is independent, additional markup language is dependent on content type switching. |
Allows markup like: Text with double__underscore__words. And it will render the double underscores within the words instead of taking them to mean to format as strong.
Solves PEP 8 line too long errors
Just fixed the merge conflict in this PR. |
@nicktimko: Thank you so much for taking the lead here and finding out the steps that are necessary to make this happen! That is super helpful! For the first item you listed, "Update the metadata spec for the Python Packaging User Guide (PyPUG)", I just submitted a PR to the PyPUG to propose a new |
Fixes the following test failure (e.g.: https://travis-ci.org/pypa/readme_renderer/jobs/148390785): ``` if "<" in expected: > assert out == expected E assert '<p>Here is s...nkey</a></p>\n' == '<p>Here is so...nkey</a></p>\n' E Skipping 734 identical leading characters in diff, use -v to show E - n class="s1">'Ruff!'</span><span class="p">)</span> E ? - E + n class="s">'Ruff!'</span><span class="p">)</span> E E - <span class="n">dog</span> <span class="o">=</span> <span class="n">Dog</span><span class="p">(</span><span class="s1">'Fido'</span><span class="p">)</span> E ? - E + <span class="n">dog</span> <span class="o">=</span> <span class="n">Dog</span><span class="p">(</span><span class="s">'Fido'</span><span class="p">)</span> E </pre> E Detailed information truncated (6 more lines), use "-vv" to show tests/test_rst.py:31: AssertionError ```
I guess the output of [Markdown](https://pypi.python.org/pypi/Markdown) changed a bit?
Hmm, why are there no Travis CI test results for this PR?
|
@dstufft: Are you able to trigger the Travis tests? |
Now that we seem to be gravitating towards making CommonMark the preferred Markdown variant in pypa/packaging.python.org#258, I should probably test how well this renders CommonMark and possibly modify it to be more CommonMark-compliant (perhaps using https://pypi.python.org/pypi/CommonMark if it does a better job of rendering CommonMark than https://pypi.python.org/pypi/Markdown ?) |
Checklist with steps that @nicktimko mentioned:
|
an option is to look at and see if rst and markdown both support some method of commenting stuff out. And then if they both have it we could use it for an advantage and look for comments containing either |
Time has shown that I haven't been able to pick this up (in part because I've changed roles and Python isn't currently a big part of my new role). Anyone else want to pick this up? |
I got an even better idea on it, why not use the vim style syntax rendering. like then you could tell it if it is Or better yet, look for HTML tags. Markdown supports HTML in the format, while rst does not support any HTML whatsoever. What if the markdown file has no HTML to embed in it? Then they can add an html comment tag that then states whatever they want to not be seen so it renders correctly in PyPi. |
We now have a way from wharehouse filters to tell appart markdown and rst via https://pypi.python.org/pypi/mdorrst, does something else blocks this PR? |
There seems to be conflicts now. |
see #51 also. |
Fixes: GH-1
Passing Travis CI build: https://travis-ci.org/msabramo/readme/builds/43547312
Cc: @dstufft, @r1chardj0n3s, @hickford, @methane