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

Tweak param signature for kbd role #1082

Merged
merged 1 commit into from
Jul 31, 2017
Merged

Tweak param signature for kbd role #1082

merged 1 commit into from
Jul 31, 2017

Conversation

tony
Copy link
Contributor

@tony tony commented Jul 13, 2017

Represent options is dict and content is list-like. This is the same practice used in sphinx-doc/docutils.

Represent ``options`` is dict and ``content`` is list-like.  This is the same practice used in sphinx-doc/docutils.
@tony
Copy link
Contributor Author

tony commented Jul 31, 2017

If this doesn't feel valuable, it's ok to close by the way, but here's my rationale:

It's de facto practice in docutils core to use this signature for roles (example). Also, the same in Sphinx (example).

A bit more python lore behind it: the {} represents an empty dict object, and [] an empty list. They help maintain type awareness in a more descriptive way than None.

@kivikakk
Copy link
Contributor

Thank you for submitting it! Happy to merge it because it makes things a little clearer; would you be able to accept the GitHub CLA first? (See 'Details' next to the failing check.)

@tony
Copy link
Contributor Author

tony commented Jul 31, 2017

Done

@kivikakk
Copy link
Contributor

Thanks! ♥️

@kivikakk kivikakk merged commit 793148b into github:master Jul 31, 2017
@tony tony deleted the patch-1 branch July 31, 2017 05:22
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