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

whitespace must precede closing comment token #148

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

billh0420
Copy link
Contributor

@billh0420 billh0420 commented Jul 31, 2023

I believe whitespace must precede the closing comment token.

The current version does the following:

 $( something $)$) yields " something $)"

 $( something junk$) yields " something junk"

 $( something $)junk$) yields " something $)junk"

 $( something ($) more $) yields error

The first three examples should continue looking for closing comment token.

The last example should yield " something ($) more "

@billh0420
Copy link
Contributor Author

Note: lines 97-107 might be clearer if written as:

if text->Js.String2.charAt(foundIdx - 1)->isWhitespace == false { // need leading whitespace
    setIdx(foundIdx+1)
}
else if !(endOfFile.contents || ch.contents->isWhitespace) { // need trailing whitespace or endOfFile
    setIdx(foundIdx+1)
}
else {
    result.contents = Some(Some(text->Js_string2.substring(~from=beginIdx, ~to_=foundIdx)))
    setIdx(foundIdx + 2)
}

This avoids advancing idx by 2 always and then moving idx back by 1 in the two exceptional cases.

I didn't test this new code: not sure if you even want it.

Would you prefer I make the change and test it?

@expln
Copy link
Owner

expln commented Aug 2, 2023

Thanks for submitting this PR. I compared how metamath-exe processes comments vs mm-lamp. Unfortunately, it appeared to be not so easy task. When I put $( something ($) more $) somewhere in the middle of an *.mm file, metamath-exe failed to parse such a file, similarly mm-lamp fails. Ideally I want mm-lamp to behave the same way as metamath-exe does. Currently they seem to behave similarly in terms of parsing comments. But according to the Metamath book, the end of a comment indeed should be preceded by a whitespace (Appendix E, Metamath Language EBNF). This could be a bug in metamath-exe too. I need more time to analyze this situation.

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