Open anderium opened 3 years ago
Oh, I just realised that there's a slight workaround for the array index assignment. The following expression changes only one element of an array:
a = [0, 1];
f(y, x, i) = y || [(i==idx) ? value : x];
fold(f, [], a);
Where idx
is the index you want to set and value
to what you want to set it.
Of course, this workaround doesn't work for setting attributes. It actually removes all attributes from the array as it creates an entirely new one, it doesn't change the array.
The fact that it creates a new array also means that it won't work if you need the changed array in another expression unless the changed array is stored as well.
Alright, I decided to try and make something that solves this, and I think I've got something that should work for most cases! I'm not sure if it solves all problems, but it at least makes array/object manipulation possible.
I've attached two patch files that can change the code because I'm not sure of the quality of my edits. With npm you can add the following to your package.json file:
{
"scripts": {
"postinstall": "patch --forward node_modules/expr-eval/bundle.js < obj-assignment-bundle.patch && patch --forward node_modules/expr-eval/index.mjs < obj-assignment-index.patch"
}
}
(This does not include a patch for the minimised version in dist/bundle.min.js
.)
The patch files are obj-assignment-bundle.txt and obj-assignment-index.txt. You might want to change the extension to .patch
to get syntax highlighting if it's supported. Github didn't allow me to upload with such an extension.
When setting a variable it's added to an object, often referenced by values
or variables
, in which the values for each variable are searched. This works when you want to create a normal variable, but when changing properties on an object what happens internally is equivalent to objectValue = new_value
instead of object[key] = new_value
. It doesn't change the variable but tries to assign the new value to a literal instead. I believe that it would actually create a variable with the value of this literal as a name!
Instead of expanding indexed variables all the way, we need to stop before the final expansion. Where I use expansion to refer to a change from object.property
or object[key]
to objectValue
. When setting a variable we then have to check whether we're dealing with a variable of which the value has to be set, or if we're dealing with an object of which we want to change a property.
To prevent the final expansion I added a new instruction type, IMEMBEREXPR
. When parsing expressions an encountered =
removes the last instruction from the internal stack. This instruction can be one of 4 things for a correct assignment:
IFUNCALL
when defining a function. This is unrelated to our problem.IVAR
when setting the value of a single variable. This was already possible.IMEMBER
when setting the property on an object. This was checked for but had problems when actually evaluating the assignment.IOP2
, specifically with value '['
, a "member expression". This was not checked for which made object[key]=value
a syntax error (parity).The first two assignments are already handled correctly. IFUNCALL
changes to an IFUNDEF
and IVAR
becomes IVARNAME
. The last two assignments require the prevention of an expansion, which is where the IMEMBEREXPR
comes in.Note Both of these are solved mostly during evaluation. During parsing the IMEMBER
instruction is replaced with IMEMBEREXPR
in the same way how IVAR
is replaced by IVARNAME
. The '['
instruction is replaced by an IMEMBEREXPR
with a single dot as value. Properties accessed with a dot can't start with a dot, so a single dot is 100% an indication of the IOP2
member expression.
In the IMEMBER
case, the property name is added to the stack with a .
prepended. Variables can't start with a dot, so when an assignment on a string starting with .
is found, we know we are actually dealing with an assignment to the property of an object. The object now at the top of the stack is the one on which this assignment should be done.
In the IOP2
member expression case, we don't need to put everything within the brackets into its own subexpression, because the object name will be resolved correctly.Note After this has been evaluated, a dot is prepended to its value for the same reason as before. The property can actually start with a dot here too and that is also handled correctly.
The actual assignment then has to check if the string starts with a .
and in those cases fetches the actual object from the stack as well as the already fetched property. Within the setVar
function we then do some checks to execute the correct assignment. There are probably only two possible combinations: nameOrObj
is a string and property
is undefined or nameOrObj
is an object and property
is defined. Even so, I added checks for the other combinations too.
See #251 for what could become more of a concern with this method.
Below the 3 functions that are different, just so that it might be easier to read than the patch files (which are stored as diffs). The evaluate
function is very long because the original is.
It's possible to extend an array using
||
as the docs say, but either I'm dumb, or it's not yet possible to use index assignment on arrays likea[1] = value
. I think it would be nice if this was possible too, perhaps as an option that can be turned off.I would try to create a PR, but I haven't any experience with good JavaScript development. I also haven't worked on software before, so I only got here through an app I'm using.
Research I did, see below for a better explanation and possible solution
I did look into the parser and I can see that it should be possible to use `.` expressions for variable assignment. The check for `IMEMBER` [here](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/parser-state.js#L153) does allow this because the `IMEMBER` type is added [here](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/parser-state.js#L322). The type added for `[i]` indexing is IOP as can be seen from the [`binaryInstruction()`](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/instruction.js#L47-49) function called [here](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/parser-state.js#L330). If I look at the [`evaluation()`](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/evaluate.js#L3) function I believe it might be kinda difficult to support this, as both ways of indexing actually push the value of the variable unto the `nstack` before the assignment. Check https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/evaluate.js#L101-L103 for `.` indexing and https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/evaluate.js#L29-L31 for `[i]` indexing. After these instructions, which don't generate references but values, have been evaluated the assignment is done: https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/evaluate.js#L19-L28 The function executed in this loop is `setVar`: https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/functions.js#L248-L251 As far as I can see this function actually only changes the list of variables, which means even using a `.` assignment wouldn't work. If I look at the console, I think I actually see which errors appear when trying to do this impossible assignment. For the string `"a=[0,1];a[1]=2"` I get the error `expected variable for assignment` as `a[1]` is neither an `IFUNCALL`, an `IVAR` or an `IMEMBER`. For the string `"a=[0,1];a.foo=2"` I get the error `invalid Expression (parity)` which indicates the `nstack` has more than one element at the end of the evaluation of the instruction.[[1]](https://github.com/silentmatt/expr-eval/blob/a556e27b93e467559f5ef6e1001b559f208f7fa0/src/evaluate.js#L117-L119) --- I totally didn't write this issue as I was figuring out the code :). So I might have missed some things or it's not clear here. However, having looked at all this, I believe it would be very difficult to add support for assignment on array indices OR attributes. It would at least require a rework of variable assignment and perhaps of indexing too.