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

Protect against indented multi-line descriptions #21

Merged
merged 2 commits into from
Feb 9, 2021
Merged

Protect against indented multi-line descriptions #21

merged 2 commits into from
Feb 9, 2021

Conversation

dgw
Copy link
Contributor

@dgw dgw commented Nov 15, 2020

Sphinx will happily e.g. turn the following into a <blockquote> when outputting HTML, because the leading indentation makes it look like one.

    list_parser = subparsers.add_parser(
        'list',
        help="List available configurations from config directory",
        description="""
            List available configurations from config directory with the
            extension ".cfg". Use option ``--config-dir`` to use a
            specific config directory.
        """)

Python's inspect.cleandoc() function was meant to handle this case, making sure that any excess indentation is stripped away.

@Exirel, this will fix the documentation rendering issue I pinged you about on IRC earlier.

dgw added a commit to sopel-irc/sopel that referenced this pull request Nov 15, 2020
One left in sopel.cli.config, which is better fixed by a small patch to
sphinxcontrib.autoprogram if the patch is accepted.
(See sphinx-contrib/autoprogram#21)
Copy link
Collaborator

@dahlia dahlia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my late response, and thank you for the contribution! Could you write a changelog for this into doc/changelog.rst?

@dgw
Copy link
Contributor Author

dgw commented Dec 3, 2020

Done. Tried to match style of previous entries, and added a new extlinks role for PRs while I was in there. 😸

@dgw
Copy link
Contributor Author

dgw commented Dec 3, 2020

Oh, I just saw the merge conflict. Would you like me to rebase, merge from master, or do nothing and let you resolve it yourself?

@dgw
Copy link
Contributor Author

dgw commented Jan 29, 2021

Decided to clean up the conflict by rebasing. Looks like CI has been failing on master for a while, so I don't expect the checks to be marked "Passed" for this PR, but it should be ready to go. I've tested it locally.

@langston-barrett
Copy link
Collaborator

This needs to be rebased after the merge of #32. @dgw If you'd like to do so, that'd be great, but if you don't want to/can't do so soon, I'll do it in a bit.

Thanks again for the contribution!

dgw added 2 commits February 9, 2021 12:59
Sphinx will happily e.g. turn the following into a <blockquote> when
outputting HTML, because the leading indentation makes it look like one.

```python
    list_parser = subparsers.add_parser(
        'list',
        help="List available configurations from config directory",
        description="""
            List available configurations from config directory with the
            extension ".cfg". Use option ``--config-dir`` to use a
            specific config directory.
        """)
```

Python's `inspect.cleandoc()` function was meant to handle this case,
making sure that any excess indentation is stripped away.
@dgw
Copy link
Contributor Author

dgw commented Feb 9, 2021

@langston-barrett Should be good to go. I happened to be in between meetings!

Maybe I should try to recruit you to help move one of my own projects to Actions 😏

@langston-barrett
Copy link
Collaborator

@dgw Excellent! Thank you!

@langston-barrett langston-barrett merged commit 2332a1a into sphinx-contrib:master Feb 9, 2021
@dgw dgw deleted the description-indentation branch February 9, 2021 22:07
dgw added a commit to sopel-irc/sopel that referenced this pull request Jun 30, 2021
One left in sopel.cli.config, which is better fixed by a small patch to
sphinxcontrib.autoprogram if the patch is accepted.
(See sphinx-contrib/autoprogram#21)
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.

3 participants