-
Notifications
You must be signed in to change notification settings - Fork 46
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
MDX compatible #53
base: master
Are you sure you want to change the base?
MDX compatible #53
Conversation
Hey @ApplebaumIan . Wow, first of all thank you so much for your kind efforts. Really appreciated. Why i didn't went for this approach on markdown is that, actually markdown doesn't have a comment syntax as in an official way. Ref: https://www.jamestharpe.com/markdown-comments/ Thus there is like n-ways. That's why went to html based commenting like other Github actions. But didn't know this would break markdown parsers, espectially mdx-parser. Now coming back to the PR. I hope you didn't manually change the dist folder. Kindly run And i think yah we do need fallback to html commenting also. Let me know if you can work on this further. If not i can take it furthur. Feel free to raise any doubts regarding this. Would recommend actually moving to a file that contains functions that gives the html matching style and comment style based on the markdown flag |
I can imagine! If I were writing this from scratch I probably would have chose HTML comments too for its ubiquity. I think I could take this on. Also I did in fact run |
…ent_style. Using ternary operator to check, with HTML comment as fall back.
…ary operator to check, with html comment as fall back.
I made the changes to the code making HTML the fall back! Just need to update the README. |
Hey @ApplebaumIan. Yup. Understood, just gave a heads up on generating the dist that's all. Now worries. Great job on implementing this feature. But i do have one suggestion. Instead of having multiple files for mdx and other one. Don't u think its better to separate just the regex and strings into seperate functions, into a file, instead of having multiple files |
Btw i just now tried html commenting in md in Docusaurus. It was working fine 🤔 |
Really? Thats strange! I'm definitely doing something a little bit rogue. I'm using the README.md file as the front page for the site. Here's the project https://github.com/ApplebaumIan/tu-cis-4398-docs-template . |
So instead of coreMdx.js add the functions to core.js. That would probably be a lot cleaner and less confusing to future contributors. I'll see If I can implement it! |
Yup. Will be much better. I'll be soon changing the entire codebase on v4 launch. Has a lot of things to consider before that though. A whole set of features coming soon. |
@akhilmhdh I believe this is ready for your review! :) |
So I figured out what causes the issue... it's not what I thought at all. It turns out Docusaurus doesn't like that there is no newline between the comment and the table... When I add the newline it actually works as intended. 🤦♂️ My immediate reaction was that it was the html comments because I'm so used to ReactJS and JSX breaking because comments aren't wrapped with |
Hey @ApplebaumIan . Glad it's not breaking any more atleast 😅 But yah ill test this out and let you know. Just wanna make sure it's not breaking anything |
🆙 |
Hey @andresin87. I have been busy with my other works. Thus not able give attention to this project for sometime. I am working on v3 for this action but may take some more time. Let me see when I get the time to review this. But mdx does work file u need to keep the indentation that's all as the previous comment suggested. So this is not blocking anything as i know. |
For my use case I'm using an Application Docusaurus, that converts markdown into ReactJS JSX. The issue with how this application originally works is that it uses HTML style comments which breaks React builds.
I'd like to propose using the markdown comment style
I have completely changed the current regex definition in JS in all areas I could find and it seems to work well for my use case, however to avoid breaking changes I could implement some sort of if statement to check the config file whether a user would like to use HTML or Markdown style comments.