josdejong / mathjs

An extensive math library for JavaScript and Node.js
https://mathjs.org
Apache License 2.0
14.41k stars 1.24k forks source link

Trailing Commas in Matrices #2968

Closed dvd101x closed 8 months ago

dvd101x commented 1 year ago

In the parser a matrix with trailing commas (final commas) has the following error

[1, 2, ]         # yields SyntaxError: Value expected (char 8)

But objects with trailing commas works fine

{a:1, b:2, }.    # yields {"a": 1, "b": 2}

It seems like a useful feature according to:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas

josdejong commented 1 year ago

Thanks for bringing this up. I think the trailing comma's accidentally work for objects.

trailing comma's are indeed sometimes handy. The article you mention says "This makes version-control diffs cleaner and editing code might be less troublesome". Ideal for lazy programmers ;). It is not a big deal for me, though I personally do not like trailing comma's that much though. I value code being easy readable for humans. Which amongst others means proper formatting, indentation, and punctuation. If my eyes read a comma, they expect something to follow that comma,

I have a slight preference to not support trailing comma's. What do you think? Would it make many people happy?

dvd101x commented 1 year ago

I think it's a nice to have

Specially when working iteratively I like the capability of adding new lines or items and it's simpler to copy and paste regardless of where the last comma is. So I like to see the results, instead seeing the error and going back to remove the extra comma.

It's a lazy way of programming but I really enjoy seeing the results and once everything works maybe I'll go back and clean up to make it more readable.

As a reference I just validated this behavior in Python, JS and Octave and they all work with trailing commas.

I guess it wouldn't be that simple as to be consistent it would also need to work with trailing semicolons. As Octave does.

[
    1, 2;
    3, 4;
]

[
    1, 2;
    3, 4;
    ,
]

[
    1, 2;
    3, 4,
]

# they all yield
#   1   2
#   3   4

At the beginning I thought it was a bug but now I know it's more like a feature request.

josdejong commented 1 year ago

OK then let's go for it and change the parser such that it allows trailing comma's and semicolons inside an array. It's not a big deal to me, just not my personal preference, but I understand it can be handy.

Anyone interested in implementing this?

gwhitney commented 1 year ago
[
    1, 2;
    3, 4;
    ,
]

This case is just weird. I would personally be OK if it wasn't allowed, along with [,] being out as a synonym for []. I mean, I guess it's not worth much extra work to avoid accepting these cases, but I certainly wouldn't try to make them OK. All the other cases shown here seem fine, although I'm personally not too fussed about them either way.

josdejong commented 1 year ago

Agree, I think it would be good to allow a trailing comma only when followed by a value/expression, and not when preceded by another comma, semicolon, or the start of an array or object: so not something like [,] or [1,2,,,].

dvd101x commented 1 year ago

I don't have a preference, I will only share some references with Octave.

[,]     # yields [ ]
[1,2,,] # syntax error

Regarding the odd cases I found a few others that works in octave:

[
  1, 2;
  3, 4 , ;
]

[
  1, 2;
  3, 4 , ;
  ,
]

# they all yield
#   1   2
#   3   4
josdejong commented 1 year ago

To add to that: In JavaScript, [,] will create an array with one empty slot (not the same as []), and [,,,] creates an Array with three empty slots, having length 3.

gwhitney commented 1 year ago

So let's just be clear: the final "ruling" on this proposal was to allow trailing commas in matrices, but not when they are "bare" - i.e. nothing before them other than a bracket or semicolon?

josdejong commented 1 year ago

Yes I think that is a good summary.

A trailing comma is only valid when preceded by a value and is followed by the end of the object/array.

Tethet-18 commented 1 year ago

OK then let's go for it and change the parser such that it allows trailing comma's and semicolons inside an array. It's not a big deal to me, just not my personal preference, but I understand it can be handy.

Anyone interested in implementing this?

Is this issue still open? I am interest in contributing to it.

josdejong commented 1 year ago

Thanks @Ayush-Kumar91221 for your offer. The issue is indeed still open: we have a concrete plan on supporting a trailing comma but it is not yet implemented.

josdejong commented 8 months ago

Support for trailing commas is now available in v12.4.0