fix: Added process to remove last empty row for examples in function references. #561
Conversation
| @@ -152,7 +152,7 @@ const correctRelativeLinksToExampleAssets = ( | |||
| return content; | |||
| } | |||
| return Array.isArray(content) | |||
| ? content.map((ex) => ex.replaceAll("assets/", "/assets/")) | |||
| ? content.map((ex) => ex.replaceAll("assets/", "/assets/").replaceAll(/\n$/g, "")) | |||
There was a problem hiding this comment.
Thanks for adding this! Two thoughts:
- Do you think we should also add it on the line below in the case when the content is just a string?
- To make things easier for other contributors coming into this in the future, maybe we should make a new function like
removeTrailingNewlines()to do this functionality and then call it in places where we callcorrectRelativeLinksToExampleAssets()? It might be unexpected, based on the name of the function, that it would include this functionality. Alternatively, maybe we can rename this to something more general, likecleanUpExampleContents?
There was a problem hiding this comment.
As you pointed out, you are right, the createRelativeLinksToExampleAssets function is indeed for modifying the links.
This was not the appropriate part of this response...
I like the idea of either removeTrailingNewlines() or cleanUpExampleContents().
( Or, I still think the policy should be to fix it in the p5.js source code... )
There was a problem hiding this comment.
I think fixing it in the p5 repo also works, although we'd maybe want to add a check in CI that there aren't trailing newlines, because I can imagine it being pretty easy to miss in code review otherwise.
But in the mean time, renaming this function or putting this change in another function that's called alongside this one is also fine by me! Thanks for finding the cause and for making these changes so far 🙂
There was a problem hiding this comment.
Thank you too.
I am happy to help improve p5.js!
resolves #559
see detail: #560 (comment)