-
Notifications
You must be signed in to change notification settings - Fork 297
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
Address issue 1004 ("\n in mo.md literally interperted in markdown mode") #1038
Address issue 1004 ("\n in mo.md literally interperted in markdown mode") #1038
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
5dbaaba
to
84de4c0
Compare
dedent-js hasn't been maintained in 10yrs, I think dendent is the correct dependency Maybe just open an issue in dedent? Alternatively, it's a trivial dep and could be moved to just utils with the patch |
I wasn't aware it wasn't maintained. I'd be fine with either of those solutions, thanks for the suggestions. |
I think this package works: https://github.com/jridgewell/string-dedent It's listed as the example in the proposal to add String.dedent to JavaScript, so would probably get polyfill support too if the proposal goes through Edit: see discussion: tc39/proposal-string-dedent#80 |
@fuenfundachtzig would you like to update this PR to use string-dedent instead? |
Sure, can do! |
Now using string-dedent. |
d56dace
to
3a693dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I added some tests in fuenfundachtzig#1
and they pass for me.
}); | ||
|
||
it("should handle space around the f=strings", () => { | ||
it("should handle space around the f-strings", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it("doesn't do this")
Looks like a quick copypaste from a different test, will update in my pr
Thank you! @fuenfundachtzig, LGTM once @dmadisetti's PR is merged. |
added r-string tests for dedent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woohoo! Thank you @fuenfundachtzig and @dmadisetti.
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.3.11-dev8 |
…de") (marimo-team#1038) * swap out dedent for dedent-js * switched to string-dedent * added r-string tests for dedent-js * Fixed misnamed test --------- Co-authored-by: dylan madisetti <[email protected]>
This only replaces the
dedent
library bydedent-js
.For details, see #1004.