Variadic grol side

[ldemailly]

So .. is a magic argument that both indicate that a function is variadic and also receives the array of optional arguments within the function

Probably need to document that last argument if an array of objects is expended for both grol and extension variadic functions, this makes:

printf=func(format, ..){print(sprintf(format, ..))}

work but printf(["%d\n", 42]) also works, ie any array not just ..

Also fixes #97

[ccoVeille]

Please add something about

1..2
1 .. 3
1 .. 4
1.. 5

To make sure how the lexer reacts with things that could be interpreted as number

And maybe

[ldemailly]

there are no range yet (1. is there already in many places)

1..2 is actually

1.
.2

but it's always been that

1 .. 2

complains about .. being undefined (unless used inside a varargs or set before)

short version is we need #56 and more documentation about the language

[ldemailly]

fuzz testing would be nice too

[ldemailly]

#101 for fancier fuzz test

[ccoVeille]

there are no range yet

Not yet,y point was to add these to tests and see what happens.

So yes,the fuzzy testing might help

[ccoVeille]

Please add the following to test

...
. ..
.. .
....

… (a Unicode ellipsis)

My idea is to make sure nothing strange could pop out from a future refactoring

[ccoVeille]

No check on isDigit before ?

See #99 (comment)

[ldemailly]

could have been "passthrough" but I already know it started with . so I don't want to redo the letter check

[ldemailly]

short version is unlike some of the other scripting language that force you to type 0.5, grol is fine with .5 (had to handle that in readNumber, including that it doesn't allow 2 dots etc... see that code and its tests)

[ccoVeille]

I would have expected something like that

Suggested change

	case '.':
	if nextChar == '.' { // DOTDOT
	l.pos++
	return token.ConstantTokenChar2(ch, nextChar)
	}
	// number can start with . eg .5
	return token.Intern(l.readNumber(ch))
	default:
	switch {
	case '.':
	if nextChar == '.' { // DOTDOT
	l.pos++
	return token.ConstantTokenChar2(ch, nextChar)
	}
	fallthrough
	default:
	switch {

or

Suggested change

	case '.':
	if nextChar == '.' { // DOTDOT
	l.pos++
	return token.ConstantTokenChar2(ch, nextChar)
	}
	// number can start with . eg .5
	return token.Intern(l.readNumber(ch))
	default:
	switch {
	default:
	if ch == '.' && nextChar == '.' { // DOTDOT
	l.pos++
	return token.ConstantTokenChar2(ch, nextChar)
	}
	switch {

[ccoVeille]

Please add tests according to this

[ldemailly]

there are tests already for .3 etc

[ldemailly]

first proposal is worse as it does a check for letter when we already know there is a .

and same for 2nd one, also all the other 2 token check are in the main switch, not in default/fallback
the start with digit or number is I think actually clearer than with the before || ch = '.'

and I don't want to have ch == '.' checked twice, I prefer to repeat token.Intern(l.readNumber(ch))

[ccoVeille]

I might be wrong in the way I reported it

But my point was that code let me think there could be a problem with

1.a
.a
1.-
4. (4 dot space)

The current code seem to me to be:

• we found a dot
• let's read the number that follows

But what comes next might not be a number

Maybe it's something that readNumber handles well. But with previous code the one in the default case, the type was checked

[ldemailly]

we found a dot let's read the number that follows

that's what the old code did too, if we agree that new code and old code are exactly the same except when .. is found we can then separate the issue of what happens with invalid numbers from the structure of the code

old (grol 0.31.0) code:

$ .a
11:49:35.063 [CRI] parser has 1 error(s)
11:49:35.063 [ERR] parser error: could not parse "." as float

same with new code and it'll be the same for all your examples as they don't involve 2 ..
(and again there are lots of test about these, like 4. space is 4.0 float and spaces are ignored, etc)

[ldemailly]

been trying to apply batch of 6 or 7 suggestions for the 3rd time already, getting "can't have more than 1 suggestion per line" bogus GitHub error

[ccoVeille]

What about adding to number tests things that could come later

1.3e6
1+6i

Adding them to test would raise errors, but then you will think about fixing later

[ldemailly]

you filled a follow up for _ and 1e6 and I filled for fuzz testing, can we merge this?

it's quite a big addition, and so is #102 and I'd like to get both in (as long as there is no terrible tech debt or bug)

[ccoVeille]

Yes, they can be merged