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

newline rule #21

Closed
johnny0914 opened this issue Sep 18, 2018 · 7 comments
Closed

newline rule #21

johnny0914 opened this issue Sep 18, 2018 · 7 comments
Assignees
Labels

Comments

@johnny0914
Copy link

johnny0914 commented Sep 18, 2018

Shouldn't '\r' be after '\r\n'? Like here

nl "newline" = !'\r''\n' / '\r' / '\r\n' { return undefined; } // catcode 5 (linux, os x, windows)

@michael-brade
Copy link
Owner

Yes, it should! Thanks for finding this 😁 I guess it's time for another unit test.

@michael-brade michael-brade self-assigned this Sep 19, 2018
@michael-brade michael-brade removed the bug label Sep 19, 2018
@michael-brade
Copy link
Owner

oh, guess I was too fast (and too smart back then when I wrote it 😜 ). The not ! is the key to make this work. But I agree, it's much more readable without the not, so I'll change it.

@johnny0914
Copy link
Author

I have tried the following in PEG.js online and I got r output for rn input.
nl = !'r''n'.* {return "n"} / 'r' .* {return "r"} / 'rn' .* {return "rn" }

The rule will never make it to 'rn' .* {return "rn" }

@michael-brade
Copy link
Owner

Ok, thinking about it again, you are right. First rule will be ignored if it finds an r, and then the second one will immediately eat the r, so the next time the first will match, never the last one. At least that was the logic I had in mind first this morning.

Now, what I don't really understand yet is that why LaTeX.js, when actually I tried to verify it, didn't interpret \r\n as a paragraph break?! I need to do some debugging...

@michael-brade
Copy link
Owner

This is really interesting. Even with the new (and correct) code, LaTeX.js/PEG.js never matches the \r\n char sequence. Only \r or \n, and if it is \r, the \n that follows it seems to be eaten by something else. This may be a bug in PEG.js. I don't know yet. So my question to you, since you seem to be using Windows: is it possible for you to use this rule

nl "newline"  = "\n"  { console.log("n") } / "\r\n" { console.log("rn") } / "\r" { console.log("r") }

and run it on a file with Windows EOLs and two or three lines, and then send me the console output?

@johnny0914
Copy link
Author

This rule seems to be correct and that's how I have done it.
I have used notepad++ to create the following lines:
latexjs
and this is the output:
rn n n rn r rn n n r

@michael-brade
Copy link
Owner

Oh my my.... /me == not so smart after all. The (second) bug is in load-fixtures.ls, which splits the lines to find the tests, and then joins them again with \n. Thank you, I'll have to fix that, too 😄

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

No branches or pull requests

2 participants