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

fix rst option list at EOF (follow-up #17442) #17638

Merged
merged 1 commit into from
Apr 4, 2021

Conversation

a-mr
Copy link
Contributor

@a-mr a-mr commented Apr 4, 2021

#17442 introduced a minor mistake when option list item is immediately finished with EOF without newline (e.g. on Windows) or when .. include directive is used.

Example:

Options:
  --deepcopy    description lastword

truncate -s 1 <file.rst>

image

Copy link
Member

@timotheecour timotheecour left a comment

Choose a reason for hiding this comment

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

LGTM, i checked it does fix the bug

@a-mr is this type of bug likely to occur in other parts of rst2html?

@timotheecour timotheecour merged commit f02e159 into nim-lang:devel Apr 4, 2021
@a-mr
Copy link
Contributor Author

a-mr commented Apr 4, 2021

@timotheecour ,

yes, IIRC there was a a few similar bugs. One of this is PR #17014 (easy part of #16990). Like here everything was good when tested manually, that is in standalone rst files hand-edited on Unix, but the bug manifested in docstrings (or on Windows) — where there is no newline character after last line. That's why it's that evasive.

Probably we need a more systematic testing for such edge cases.

Now I think that there is probably a point in adding newline manually to the seq of tokens (tkIndent before tkEof), that would solve most, if not all, such problems.

@timotheecour
Copy link
Member

Now I think that there is probably a point in adding newline manually to the seq of tokens (tkIndent before tkEof), that would solve most, if not all, such problems.

that's exactly what i was going to suggest :) maybe followup PR?

@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Apr 4, 2021
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: followup needed remove tag once fixed or tracked elsewhere
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants