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

scheme language issues #1451

Closed
4 tasks done
jcubic opened this issue Jun 23, 2018 · 20 comments
Closed
4 tasks done

scheme language issues #1451

jcubic opened this issue Jun 23, 2018 · 20 comments

Comments

@jcubic
Copy link
Contributor

jcubic commented Jun 23, 2018

So I found few issues:

  • string don't start with ' in scheme ' is for quote (you can quote names, which become symbols or lists) (Scheme: Improvements #2263)
    solution: remove |'[^('\s]* from string regex
  • lambda,define have first parameter mark as function invocation (lambda (foo bar) (concat foo bar)) foo is mark as function, I've try to came up with regex but was not able to find one. (Scheme: Improvements #2263)
  • function without argument is not marked as function
    solution: /(\()[^\s()]*(?=[\s)])/, added (?=[\s)]) instead of (?=\s) (fix function without arguments in scheme language #1463)
  • numbers that don't have space before them are not marked as numbers
    solution: optional space in the beginning of number regex (\s?|[()])

Wanted to create PR but don't know how to fix function with define and lambda.

@Golmote
Copy link
Contributor

Golmote commented Jul 6, 2018

Hi @jcubic. You can definitely submit a PR fixing 3 bugs out of 4, and you are very welcome to do so!

Regarding the lambda case, it might be tricky. The negative lookbehind suggested in #1445 might come in handy to solve such a case.

@jcubic
Copy link
Contributor Author

jcubic commented Jul 7, 2018

It seems that numbers are correct and string regex don't match single quote string but symbols, so only function without arguments is not marked as function, will create PR.

@jcubic
Copy link
Contributor Author

jcubic commented Jul 7, 2018

@jcubic
Copy link
Contributor Author

jcubic commented Oct 8, 2018

Found another 2 issues with scheme mode:

  • strings in scheme can be multiline (Scheme: Minor improvements #1814)
  • function name that start as keyword like define is not mark as function (to fix this the regex first need to check for function name and after if fail it need to check for keyword like define or lambda) (Scheme improvements #1556)

Example:

(display (list->array '(1 2 3 "foo
bar
baz")))
(list 1 2 3)

on GitHub multiline strings works and list->array don't have list highlighted (on GitHub scheme syntax function names are black)

  1. issue can bee fixed by changing regex (if PrismJS don't split text with newlin before processing)
  2. issue may be problematic if keyword is processed by prism before processing functions (maybe using negative look ahead with space or ) will solve the issue).

@jcubic
Copy link
Contributor Author

jcubic commented Oct 8, 2018

I can create PR with the fix. Last one was not merged, @Golmote did you notice it?

@RunDevelopment
Copy link
Member

@jcubic
Number 2 is already addessed in #1556.
(And Prism doesn't split the code by lines before processing.)

@jcubic
Copy link
Contributor Author

jcubic commented Mar 24, 2020

1 in 1+2 is mark as number but this is symbol, in scheme I think that tokens are delimited only by /[\s()]/, and probably also by special symbols like , ` ,@ and '

@jcubic
Copy link
Contributor Author

jcubic commented Mar 24, 2020

I think that this do the trick for last number issue:

/([\s()])[-+]?\d*\.?\d+(?:\s*[-+]\s*\d*\.?\d+i)?([\s()])/

([\s()]) at the end, instead of \b, same as at the begining, but I need to run the tests to be sure that this don't break anything.

@RunDevelopment
Copy link
Member

@jcubic I want to close this issue with #2263 but I have two questions:

lambda, define have first parameter mark as function invocation [...]

Can you please give an example with define?

numbers that don't have space before them are not marked as numbers

An example of this one as well, please.


Also, I changed your comments a bit, so that I could track which parts of the issue were already done. Please tell me in case I missed anything.

@RunDevelopment
Copy link
Member

RunDevelopment commented Mar 24, 2020

I forgot to mention this: case-lambda.
I don't think that we will ever find a way to not mark these arguments as functions. I'll make an entry on the known failures page.

@jcubic
Copy link
Contributor Author

jcubic commented Mar 24, 2020

The for numbers the regex need to be /([\s()])[-+]?\d*\.?\d+(?:\s*[-+]\s*\d*\.?\d+i)?(?=[\s()])/. look ahead at the end otherwise it will match 10)

I have no idea what the issue was with numbers without space. Numbers always need to have space, maybe this "(number? `10)" biwascheme return true for this even that it's quote. I think this can be ignored.

@RunDevelopment
Copy link
Member

Maybe rational numbers? Anyway, I'll just ignore it.

@jcubic
Copy link
Contributor Author

jcubic commented Mar 28, 2020

Found another issue, rational numers literals are ok but not complex one:

Some of them are marked as numbers other not:

(list 10i +10i -10i 10.10i 10+10i 10.10+10.10i 10-10i 10e+10i 10+10e+10i)

(the are all valid complex numbers acording to Kawa scheme intereter that I'm testing this agains).

it can be ("+" "-") number ("+" "-") number "i", where number can be float or integer, float can use scientific notation and first 3 items are optional.

I can help with regex since I'm now working on my Scheme interpreter in JavaScript and I also need to parse those correctly.

@RunDevelopment
Copy link
Member

Fixed this in #2263.

I also removed the optional spaces around the [+-] linking the rational and imaginary parts of a complex number. 2 + 4i should be interpreted as 3 symbols, right?

@RunDevelopment
Copy link
Member

Also, I found this grammar of Scheme numbers. Should Prism support this as well or is the current pattern sufficient?

@jcubic
Copy link
Contributor Author

jcubic commented Mar 28, 2020

Those works in Kawa and Guile:

(list #xAFD #b1110011 #o777)

Guile is more compilant it have:

#i#x10

as inexact (float) hex,

or

#i#x10+10i

float hex complex

Guile also works with this:

#b10+10i

binary complex number. I think since Prism is not a parser I think that it would be ok to add:

(?:#[bxieo]){1,2} in front of number (and digits need to be [0-9A-F]), it will mach what it should but simply will not check if the value is #b (binary) and only 0 1 I think that this simplification is ok.

@jcubic
Copy link
Contributor Author

jcubic commented Apr 6, 2020

I think I know what was the issue with numbers "10" is not marked as number. so beginning of regex need to be ([(\s]|^) to match beginning of the strings same probability need to be done at the end.

I can try update the regex and create unit tests.

@RunDevelopment
Copy link
Member

The ^ in ([(\s]|^) is really dangerous because it doesn't actually refer to the start of the string but the end of the previous token. This is because Prism repeatedly splits the input string under the hood.
So removing that one false negative will likely result in many false positives.

@RunDevelopment
Copy link
Member

Binary, octal, and hexadecimal numbers are now supported in #2263.

@RunDevelopment
Copy link
Member

#2263 is merged and I think it addresses all issues here. Feel free to reopen this in case I missed something.

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

4 participants