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 Description-Content-Type field #258

Merged
merged 16 commits into from May 19, 2017

Conversation

Projects
None yet
@msabramo
Contributor

msabramo commented Jul 29, 2016

@msabramo msabramo force-pushed the msabramo:description-content-type branch 3 times, most recently from b9195d8 to c4cb911 Jul 29, 2016

Add Description-Content-Type field
This allows specifying the content type of the distribution's
description (e.g.: `text/plain`, `text/x-rst`, `text/markdown`).

@msabramo msabramo force-pushed the msabramo:description-content-type branch from c4cb911 to fcd83ac Jul 29, 2016

@msabramo msabramo referenced this pull request Jul 29, 2016

Closed

Add Markdown support #3

@nicktimko

This comment has been minimized.

Contributor

nicktimko commented Jul 29, 2016

Bikeshedding: CommonMark? Can that be the default?

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

Maybe. I've never used CommonMark; in fact, I hadn't heard of it until you mentioned it. Have a Python implementation that you like? https://pypi.python.org/pypi/CommonMark perhaps?

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

How is CommonMark different than the original Markdown? Why do you prefer it?

@nicktimko

This comment has been minimized.

Contributor

nicktimko commented Jul 30, 2016

CommonMark (http://commonmark.org/) is rigidly defined unlike Markdown which has bits of ambiguity (true "original" Markdown.pl is properly broken in some nesting cases). It improves a bunch of quirks, like spaces around lists, and standardizes some of the more convenient parts of GFM like fenced code blocks.

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

http://commonmark.org/ sounds interesting and has some big names behind it. I'm intrigued. I think I'll take a closer look and consider making CommonMark the default.

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

BTW, the only reason that I made original Markdown the default is because it's listed as the initial variant in RFC 7763 at https://tools.ietf.org/html/rfc7763#section-6.1.4, but then CommonMark is referenced in RFC 7764 at https://tools.ietf.org/html/rfc7764#section-4.5 (which I hadn't noticed before), so I guess nothing prevents it from being the default.

Part of me thinks that because there are so many competing Markdown variants and so much confusion around Markdown, that perhaps there should be no default and we should require the variant to be specified. Thoughts on that?

@ncoghlan

This comment has been minimized.

Member

ncoghlan commented Jul 30, 2016

The overall default on the server side needs to be UTF-8 ReST (as in the current PR) for compatibility with existing projects that already correctly use ReST formatting.

However, when the media type is given as text/markdown, I'd actually suggest using either GFM (GitHub Flavored Markdown) or CommonMark (where GitHub are one of the backing organisations) as the default, as one of the more common reasons for using Markdown in README files these days is as a nice user-friendly front page for a GitHub repo.

So using CommonMark as the default (including falling back to it if asked to process an unknown variant) makes sense to me.

- ``text/markdown``

There is one required parameter called ``charset``, to specify whether the text
is UTF-8, ASCII, etc.

This comment has been minimized.

@ncoghlan

ncoghlan Jul 30, 2016

Member

I'd make charset optional, defaulting to UTF-8 - that way text/markdown is sufficient to get Markdown rendering.

This comment has been minimized.

@msabramo

msabramo Jul 30, 2016

Contributor

My first instinct was to default to charset to UTF-8.

But then I saw this in RFC 7763 section 2:

     charset: Per Section 4.2.1 of [RFC6838], charset is REQUIRED.
     There is no default value because neither [MDSYNTAX] nor many
     popular implementations at the time of this registration do
     either.

That said, we can disagree with that RFC and do what we want, but I thought deferring to previous work would be a good start, since they probably had reasons and discussions around this.

Intuitively, it seems to me to be a good idea to simply default to UTF-8. I'm not sure why the RFC didn't simply do that. I wonder if they've thought of something that we haven't?

This comment has been minimized.

@msabramo

msabramo Jul 30, 2016

Contributor

OK, I went ahead and made charset optional and defaulted it to UTF-8.

d454938

This comment has been minimized.

@ncoghlan

ncoghlan Jul 31, 2016

Member

In the general case, I agree that there's no sensible universal default, but Python project descriptions aren't the general case - they're a specific use case. That lets us make assumptions in the name of ease-of-use that wouldn't be valid given a broader context.

@ncoghlan

This comment has been minimized.

Member

ncoghlan commented Jul 30, 2016

+1 from me for the general idea, but what you have so far is missing guidance on expected behaviour for tools reading the field to render the description when:

  • they don't recognise the content type
  • they don't recognise the charset
  • they don't recognise the specific variant

Those additions would be SHOULD/MAY type guidelines rather than MUST requirements, but I think we still want to give some thought to how PyPI should handle those cases, and how those considerations can be turned into general recommendations.

msabramo added a commit to msabramo/setuptools_old that referenced this pull request Jul 30, 2016

WIP: Add new long_description_content_type kwarg
This is used to populate the newly proposed `Description-Content-Type`
field, that is proposed in
pypa/python-packaging-user-guide#258.

msabramo added a commit to msabramo/setuptools that referenced this pull request Jul 30, 2016

WIP: Add new long_description_content_type kwarg
This is used to populate the newly proposed `Description-Content-Type`
field, that is proposed in
pypa/python-packaging-user-guide#258.
@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

Added mentions of CommonMark in 774d091.

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 30, 2016

Made CommonMark the default variant for Markdown in 33dc8c5.

@ncoghlan

This comment has been minimized.

Member

ncoghlan commented Jul 31, 2016

Latest draft looks good to me, but I'll defer to @dstufft for actual sign-off.

While I don't expect this idea to be controversial, we should also give folks a chance to discuss the idea on distutils-sig before committing to it.

@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 31, 2016

@ncoghlan: I'm still planning to add some info on expected behavior when things are omitted or invalid, as you suggested in #258 (comment).

But certainly, I'd love to have @dstufft and anyone else's comments.

One parameter is called ``charset``; it can be used to specify whether the
character set in use is UTF-8, ASCII, etc. If ``charset`` is not provided, then
it is recommended that the implementation (e.g.: PyPI) treat the content as
UTF-8.

This comment has been minimized.

@pfmoore

pfmoore Jul 31, 2016

Member

Maybe move the sentence "If charset is not provided..." down below? (See next comment)

This comment has been minimized.

@msabramo

msabramo Jul 31, 2016

Contributor

Does e4b39db help?

This comment has been minimized.

@ncoghlan

ncoghlan Jan 31, 2017

Member

I agree with @pfmoore that this can be dropped in favour of the more concise specification below.


If a ``Description-Content-Type`` is not specified, then the assumed content type
is ``text/x-rst; charset=UTF-8``.

This comment has been minimized.

@pfmoore

pfmoore Jul 31, 2016

Member

Maybe include the case of a missing charset here? Something like

If a Description-Content-Type is present, but has no charset specified, then the character set is assumed to be UTF-8

I'm not sure I see much benefit in distinguishing between "charset isn't given but the implementation should treat it as UTF-8" and "when charset isn't given, it defaults to UTF-8".

This comment has been minimized.

@msabramo

msabramo Jul 31, 2016

Contributor

@pfmoore Does e4b39db address your concern?

This comment has been minimized.

@pfmoore

pfmoore Jul 31, 2016

Member

Yep, looks fine to me, thanks.

@pfmoore

This comment has been minimized.

Member

pfmoore commented Jul 31, 2016

Overall, LGTM (one minor comment in the diff).

Clarify how to treat missing/invalid values
for `Description-Content-Type` (and `charset` and `variant`)

as suggested by @ncoghlan at
#258 (comment)
@msabramo

This comment has been minimized.

Contributor

msabramo commented Jul 31, 2016

e4b39db: Clarified how to treat missing/invalid values for Description-Content-Type (and charset and variant) as suggested by @ncoghlan at #258 (comment)

@dstufft

This comment has been minimized.

Member

dstufft commented May 19, 2017

Sadly I'm not at PyCon, I just haven't gotten a chance to circle back around to this yet :(

@dstufft dstufft merged commit 42c2678 into pypa:master May 19, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@msabramo

This comment has been minimized.

Contributor

msabramo commented May 19, 2017

Yay, I see @dstufft merged this. Great news!

So what do we need to do next?

@dstufft

This comment has been minimized.

Member

dstufft commented May 19, 2017

Probably implement support in setuptools, then wheel, then twine, then PyPI/Warehouse.

@nicktimko

This comment has been minimized.

Contributor

nicktimko commented May 20, 2017

Would it be better to have the server (PyPI/Warehouse) accepting it before the clients? Then newer clients wouldn't get errors if they support what the server doesn't

@ncoghlan

This comment has been minimized.

Member

ncoghlan commented May 20, 2017

I think there are a few aspects with a required order, then some that can happen in arbitrary order:

  1. Check that pip will ignore the new field at install time. If not, update it, and document the minimum required version implied by using the new field. Also make note of whether or not easy_install will be able to install packages using the new field (we don't have to fix that if it does, but we should document the answer)
  2. Update PyPI/Warehouse to allow the new field (if they don't already)
  3. Update setuptools and the other build systems to allow setting the new field, and PyPI/Warehouse to actually pay attention to it

(We should probably document this somewhere on pypi.io as a standard practice, since it applies to any interoperability metadata changes we make)

ncoghlan added a commit to ncoghlan/python-packaging-user-guide that referenced this pull request Jun 24, 2017

Clarify how to treat missing/invalid values
for `Description-Content-Type` (and `charset` and `variant`)

as suggested by @ncoghlan at
pypa#258 (comment)

ncoghlan added a commit to ncoghlan/python-packaging-user-guide that referenced this pull request Jun 24, 2017

Require UTF-8, exempli gratia -> id est
The former subject to a little agreement by others: pypa#258 (review)

ncoghlan added a commit to ncoghlan/python-packaging-user-guide that referenced this pull request Jun 24, 2017

Remove extra backtick
that was causing incorrect rendering

pointed out by @ekohl in pypa#258 (comment)

ncoghlan added a commit to ncoghlan/python-packaging-user-guide that referenced this pull request Jun 24, 2017

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258.

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 3, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 24, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Jul 24, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Aug 28, 2017

Add new long_description_content_type kwarg
This is used to populate the new `Description-Content-Type` field.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

di added a commit to di/setuptools that referenced this pull request Aug 28, 2017

Add test_long_description_content_type
Test that specifying a `long_description_content_type` keyword arg to
the `setup` function results in writing a `Description-Content-Type`
line to the `PKG-INFO` file in the `<distribution>.egg-info` directory.

`Description-Content-Type` is described at
pypa/python-packaging-user-guide#258

@ncoghlan ncoghlan referenced this pull request Dec 4, 2017

Closed

Metadata 1.3 PEP #1

@wimglenn

This comment has been minimized.

Contributor

wimglenn commented Dec 5, 2017

@ncoghlan or anyone else who knows about this, could you please explain how to get it working properly? I added in setup.cfg:

[metadata]
long_description_content_type = text/x-rst; charset=UTF-8

This is syntax documented as supported in setuptools (here). But it doesn't seem to be respected or used for anything - the generated metadata in the distribution always just shows "UNKNOWN". Is the correct value getting dropped in the installation at some stage?

I use the version of setuptools v38.2.4, and I use "python setup.py bdist_wheel" to build the dist.

@di

This comment has been minimized.

Member

di commented Dec 5, 2017

Hi @wimglenn, there's an open PR to setuptools to fix this: pypa/setuptools#1207

@wimglenn

This comment has been minimized.

Contributor

wimglenn commented Dec 6, 2017

@di ah, yes, that’s exactly the spot I ended up at in pdb when I was looking at it. Thanks!

@scls19fr scls19fr referenced this pull request May 11, 2018

Merged

no setuptools-markdown #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment