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

Slate transform does not roundtrip when the text uses softbreaks #44

Closed
jeromesimeon opened this issue Oct 3, 2019 · 6 comments
Closed
Assignees

Comments

@jeromesimeon
Copy link
Member

jeromesimeon commented Oct 3, 2019

Describe the bug
Translating from markdown to slate and back sometimes introduces additional new lines.

To Reproduce

For instance on the attached sample.md:
sample.txt

$ cat sample.md 
## Fixed rate loan

This is a _fixed interest_ loan to the amount of 100000
at the yearly interest rate of 2.5%
with a loan term of 15,
and monthly payments of {{I'm not sure which amount right now}}
$ markus normalize --sample sample.md --slate
12:34:44 - info: 
Fixed rate loan
----

This is a *fixed interest* loan to the amount of 100000

at the yearly interest rate of 2.5%

with a loan term of 15,

and monthly payments of {{I'm not sure which amount right now}}
@jeromesimeon jeromesimeon self-assigned this Oct 3, 2019
@jeromesimeon
Copy link
Member Author

This issue is most likely related to softbreaks which gets translated into paragraphs. See also accordproject/template-studio-v2#147

@jeromesimeon jeromesimeon changed the title Slate transform does not roundtrip in some cases Slate transform does not roundtrip when the text uses softbreaks Nov 6, 2019
@dselman
Copy link
Sponsor Contributor

dselman commented Nov 6, 2019

I think you are correct: https://github.com/accordproject/markdown-transform/blob/master/packages/markdown-slate/src/ToSlateVisitor.js#L235

We may need some metadata on the Slate para to identify it as a "soft break"?
https://github.com/accordproject/markdown-transform/blob/master/packages/markdown-slate/src/slateToCiceroMarkDom.js#L64

@jeromesimeon
Copy link
Member Author

jeromesimeon commented Nov 6, 2019

I'm not sure what is the right way to address this. A couple of possible directions:

  1. Preserve softbreaks in the Slate DOM and delay the new line creation until rendering (this might be useful if we want to allow users to control that when editing)
  2. Turn softbreaks into a new line folded in the enclosing paragraph (a bit like the HTML transform), but that means we need to recover those when going from Slate DOM back to Cicero DOM.

As I'm writing it down, I think like option 1. better but it might be more work (since this is a new kind of Slate node to handle in the editor)... There might be other options to handle this that I'm not thinking about.

@dselman
Copy link
Sponsor Contributor

dselman commented Nov 6, 2019

Yes, I think I like (1) better as well. E.g. is would allow CTRL-ENTER in the editor to insert a softbreak.

FYI, here is what the CommonMark spec says: https://spec.commonmark.org/0.22/#soft-line-breaks

@jeromesimeon
Copy link
Member Author

Yes, I think I like (1) better as well. E.g. is would allow CTRL-ENTER in the editor to insert a softbreak.

FYI, here is what the CommonMark spec says: https://spec.commonmark.org/0.22/#soft-line-breaks

Yes. That makes sense. @mttrbrts encountered this as well on the HTML side when he looked at pdf rendering, so he may have some thoughts?

@dselman dselman assigned dselman and unassigned jeromesimeon Nov 7, 2019
@jeromesimeon
Copy link
Member Author

This bug was fixed in #113

bash-3.2$ markus normalize --sample sample.md --slate
15:02:01 - info: 
Fixed rate loan
----

This is a *fixed interest* loan to the amount of 100000

at the yearly interest rate of 2.5%

with a loan term of 15,

and monthly payments of {{I'm not sure which amount right now}}
bash-3.2$ ./packages/markdown-cli/index.js normalize --sample sample.md --slate
15:02:10 - info: 
Fixed rate loan
----

This is a *fixed interest* loan to the amount of 100000
at the yearly interest rate of 2.5%
with a loan term of 15,
and monthly payments of {{I'm not sure which amount right now}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants