-
Notifications
You must be signed in to change notification settings - Fork 12
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
Bug / Idea.... Pipe breaks links #12
Comments
Unfortunately it's an issue in Kramdown, see gettalong/kramdown#431 / jekyll/jekyll#2818 / https://stackoverflow.com/questions/23751917/how-do-you-disable-tables-in-kramdown and it seems to be a wontfix. Creating a remark-lint plugin for this would make sense IMO. |
I've started putting together a plugin... It's been a while since I've done node - just figuring out where it should go... Should also check if there are other things that could cause rendering issues... |
Hi @vhf - I've put together a plugin based on taking apart the other plugins that are available, and pretty much copying what was there. I have uploaded it into a repo - not sure how to proceed https://github.com/computamike/remark-link-escape I could upload it as an NPM package, and then it would just be a case of referencing that from within the free-programming-books-lint? |
I've created an NPM package - was able to upload from Command Line - but was hoping to publish from GitHub Action - getting an error so will need to look into it. |
Good work! I think the naming of this package is a bit too broad, making it more explicit (and prefixing it with Inspired by how other plugins are named here: https://github.com/remarkjs/remark-lint#list-of-external-rules, something like After that I'll help you fix your plugin, you don't need to count lines and run regexs over the whole content since the plugin gets an AST. Instead, we should visit the link nodes and check the link title part only. Here's a similar plugin visiting link nodes: https://github.com/vhf/remark-lint-no-url-trailing-slash/blob/master/lib/resource-url.js |
Thanks, @vhf - these are great points. I've unpublished the package from npm - great point about the AST - I'll have a go at your suggestions. |
Hi @vhf, I've gone ahead and made the changes you suggested. I'm a bit dubious about the visit within a visit that I have written. I'm not sure if there's a better way of accomplishing this? I also raise a message per pipe that exists - not sure if I should only raise a single message for a link, or multiple messages for each pipe within a link? - Most instances that were fixed were single instances within a link. |
Your visit within a visit is fine: visiting every text node in every link and nothing else. 👍
That's up to you, both options make sense in my opinion. |
hi @vhf - sorry i've been a bit slow. I have build a v0.1 package, and published it to GitHub. |
Links in the form :
[ xxx | yyy](zzz)
break when rendered using Jekyll / Github Pages.
I found this issue #5176 when looking through the screencasts page :
I submitted a PR to fix this issue in all the files I found it in and used the following regular expression to identify it :
^(.*)(\[)(.*)(\|)(.*)(\])
This might need some work - but it seems that the linter might be a good place to highlight if this issue were to occur again.
If I get a chance I'll have a look at implementing this - but I thought it would be a good idea to document this issue here incase someone on the team has more opportunity to fix this than I do.
The text was updated successfully, but these errors were encountered: