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

Fixes unintentional newline characters within lists with paragraphs #93

Merged
merged 1 commit into from
Oct 31, 2021
Merged

Fixes unintentional newline characters within lists with paragraphs #93

merged 1 commit into from
Oct 31, 2021

Conversation

diogoosorio
Copy link
Contributor

This commit aims to fix a problem when parsing markup similar to:

[8] pry(main)> ReverseMarkdown.convert("<ul><li> <p>a</p></li></ul>")
=> "-  \n\na\n\n"

Note that the list item has newlines between the markdown list item and
the actual text.

If you remove the space between the list item tag and the paragraph tag:

[7] pry(main)> ReverseMarkdown.convert("<ul><li><p>a</p></li></ul>")
=> "- a\n\n"

Reverse markdown appears to assume what believe to be the intended
behaviour. There was already a test in place to account for this situation,
but it was xit'ed.

The proposed patch is to ask Nokogiri to find the first child element of
the list, instead of the first child (which might include stuff like a
newline).

This commit aims to fix a problem when parsing markup similar to:

```ruby
[8] pry(main)> ReverseMarkdown.convert("<ul><li> <p>a</p></li></ul>")
=> "-  \n\na\n\n"
```

Note that the list item has newlines between the markdown list item and
the actual text.

If you remove the space between the list item tag and the paragraph tag:

```ruby
[7] pry(main)> ReverseMarkdown.convert("<ul><li><p>a</p></li></ul>")
=> "- a\n\n"
```

Reverse markdown appears to assume what believe to be the intended
behaviour. There was already a test in place to account for this situation,
but it was `xit`'ed.

The proposed patch is to ask Nokogiri to find the first child element of
the list, instead of the first child (which might include stuff like a
newline).
@diogoosorio diogoosorio marked this pull request as ready for review October 8, 2020 15:58
@diogoosorio
Copy link
Contributor Author

Aims to fix #92 and forem/forem#10664

@diogoosorio
Copy link
Contributor Author

Ping? 😄

@xijo xijo merged commit 2eab8dd into xijo:master Oct 31, 2021
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