Skip to content

Conversation

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented May 9, 2024

Summary

Before this patch, string defaults are formatted without quotes around them.
However, if the default is a tuple or list of strings, quotes will be included:

default="a" ==> :default: ``a``
default=("a",) ==> :default: ``('a')``

This is a slightly annoying inconsistency, and leads to potential for confusion
between default="0" and default=0. Most bothersome is that in the case of an
empty string we get:

:default: ````

Which makes docutils angry:

CRITICAL: Unexpected section title or transition: ````

This fixes the trouble by formatting the repr of the default.

On top of #136.

Tasks

  • Added unit tests
  • Added documentation for new features (where applicable)
  • Added release notes (using reno)
  • Ran test suite and style checks and built documentation (tox)

@hoodmane hoodmane force-pushed the empty-string-default branch 2 times, most recently from 6e2ae93 to 88e1a84 Compare May 9, 2024 14:21
@hoodmane hoodmane changed the title Empty string default Fix rendering of string-valued defaults May 9, 2024
@hoodmane
Copy link
Contributor Author

hoodmane commented May 9, 2024

There's still a CRITICAL: Unexpected section title or transition if a multiple option has an empty list default.

hoodmane added a commit to hoodmane/pyodide that referenced this pull request May 9, 2024
hoodmane added a commit to pyodide/pyodide that referenced this pull request May 9, 2024
@stephenfin
Copy link
Member

stephenfin commented May 14, 2024

You can rebase this now that #136 is merged too.

Before this patch, string defaults are formatted without quotes around
them. However, if the default is a tuple or list of strings, quotes will
be included:

  default="a" ==> :default: ``a``
  default=("a",) ==> :default: ``('a')``

This is a slightly annoying inconsistency, and leads to potential for
confusion between `default="0"` and `default=0`. Most bothersome is that
in the case of an empty string we get:

  :default: ````

Which makes docutils angry:

  CRITICAL: Unexpected section title or transition.

This fixes the trouble by formatting the repr of the default.
@stephenfin stephenfin force-pushed the empty-string-default branch from 3354956 to 4953efe Compare May 15, 2024 12:27
@stephenfin stephenfin merged commit 7f29172 into click-contrib:master May 15, 2024
@hoodmane hoodmane deleted the empty-string-default branch May 16, 2024 13:50
@hoodmane
Copy link
Contributor Author

Thanks for maintaining sphinx-click @stephenfin!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants