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

fix parsing of newline in RAW-LINE #60

Merged
merged 12 commits into from
Jul 25, 2023
Merged

fix parsing of newline in RAW-LINE #60

merged 12 commits into from
Jul 25, 2023

Conversation

melisgl
Copy link
Contributor

@melisgl melisgl commented Jul 12, 2023

Previously, carriage return characters were not allowed in RAW-LINE. Since NEWLINE tested for linefeed or carriage return plus linefeed, this resulted in carriage return characters without a following linefeed causing havoc. See the added test.

melisgl pushed a commit to melisgl/3bmd that referenced this pull request Jul 12, 2023
Previously, carriage return characters were not allowed in RAW-LINE.
Since NEWLINE tested for linefeed or carriage return plus linefeed,
this resulted in carriage return characters without a following
linefeed causing havoc. See the added test.

3b#60
@melisgl
Copy link
Contributor Author

melisgl commented Jul 12, 2023

This pull request now has fixes for #58, #59 and #60.

@melisgl
Copy link
Contributor Author

melisgl commented Jul 12, 2023

PAX needed small updates to follow up on these changes, but the output is now identical.

melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 12, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 12, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
@3b
Copy link
Owner

3b commented Jul 12, 2023

will try to get to this soon, when i have more time to think about it. Looks reasonable from a quick skim though, aside from I'd probably name the *chars-to-escape... vars as *inline-chars-to-escape* and *block-chars-to-escape* or similar.

I think there may be a few more edge cases to deal with to support other blocks correctly, though I'll probably merge this first either way.

In particular I wonder about horizontal rules (--- * * * - - - - - etc..), setext headers (also --- depending on context, and ==== etc), and numbered lists. Looks like numbered lists might work already since it preserves the \ in the parse, and printing \-- for -- at beginning of the line might be OK since the extra \ gets dropped on parse in that case, but needs more thought than i have time for now.

(If you want to work on it further before i get to it, might rebase onto the rc branch, so it can test against blocks with more complicated indentation as well.)

@melisgl
Copy link
Contributor Author

melisgl commented Jul 12, 2023 via email

Gabor Melis added 2 commits July 12, 2023 17:50
Previously, carriage return characters were not allowed in RAW-LINE.
Since NEWLINE tested for linefeed or carriage return plus linefeed,
this resulted in carriage return characters without a following
linefeed causing havoc. See the added test.

3b#60
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 12, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 12, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
@melisgl
Copy link
Contributor Author

melisgl commented Jul 12, 2023

Seems to work fine after the rebase. Also tested along with a host of unrelated changes (except for "follow up on changes on 3bmd's rc branch"), PAX's dref branch (https://github.com/melisgl/mgl-pax/tree/dref).

I'm unlikely to emerge from my PAX todo list. So this may be as far as I'll go on the 3bmd side of things.

Gabor Melis added 3 commits July 13, 2023 12:11
That is, when a # character would be printed right after an MD-INDENT
and we are not in code.

3b#58
... for ease of embedding latex.

3b#59
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 13, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 13, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
@melisgl
Copy link
Contributor Author

melisgl commented Jul 14, 2023

Well, I had to come back to this to fix the escaping of < and &. It think commit fbd1075 makes escaping correct for all special chars. It tries to use escapes economically, but utterly fails to do that with < and &. I guess, correctness first?

The escaping is now print/parse consistent but overly aggressive in
that it escapes "<->" as "\\<->" and "&KEY " as "\\&KEY ", which is
not necessary.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 14, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 16, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 16, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 20, 2023
Previously, a single carriage return not followed by a newline (as
would be the case on Windows) would sometimes make the variable's
docstring be presented as a verbatim block.

This fix also needs 3b/3bmd#60 from 3BMD.
melisgl added a commit to melisgl/mgl-pax that referenced this pull request Jul 20, 2023
... and don't escape > because it's never necessary.

Needs 3b/3bmd#60 from 3BMD.
@3b
Copy link
Owner

3b commented Jul 25, 2023

Doesn't look like I'll have any energy for poking at edge cases, and seems like current state of the PR is an improvement in general even if there are some remaining, so will merge it and think about it more if we run into any specific problems later.

@3b 3b merged commit 0cc2f86 into 3b:master Jul 25, 2023
1 check passed
3b pushed a commit that referenced this pull request Jul 25, 2023
Previously, carriage return characters were not allowed in RAW-LINE.
Since NEWLINE tested for linefeed or carriage return plus linefeed,
this resulted in carriage return characters without a following
linefeed causing havoc. See the added test.

#60
This was referenced Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants