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 nil output <a> name attribute conversion #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

logwolvy
Copy link

The attribute described(link below) was being converted incorrectly for certain pages I tried.
https://www.w3schools.com/tags/att_a_name.asp

I created this gist based on the page I was trying to convert and before fix I was getting -

'<a name="sp">&nbsp;&nbsp;</a>' # => [ ]
'<a name="serno">87471742</a>' #> [87471742]

I admit this is really badly written non-html5 doc, but I really need to convert this for a project in production. After this fix, these conversions are fine.
Kindly, merge this fix. Thanks!

@soundasleep
Copy link
Owner

Thank you for the PR! Can you please include a test case that illustrates the change?

@logwolvy
Copy link
Author

Hi @soundasleep, I think we shouldn't convert 'a' tags to markdown like syntax when they have name attributes?

Example

# Expected Behavior
<a name="serno">87471742</a> #> 87471742
# Actual Behavior
<a name="serno">87471742</a> #> [87471742]

The actual behavior works well when the 'a' tag has an href attribute but for name attributes, it shouldn't add the square brackets. What do you think?

@soundasleep
Copy link
Owner

I think your proposal is totally fine. If an <a> tag does not have any href, then it shouldn't display as a link. Can you write this into your commit and add a test case (or two) showing the difference, and get the test to pass on travis-ci? Thank you!

@soundasleep soundasleep force-pushed the master branch 7 times, most recently from 956ec37 to 3330c3f Compare February 14, 2019 02:44
gendosu added a commit to pocake/html2text_ruby that referenced this pull request Apr 25, 2023
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