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

Deleting the last key by negative index does not work; deleting keys by value deletes all of them #183

Open
JonnyOThan opened this issue Mar 2, 2024 · 6 comments

Comments

@JonnyOThan
Copy link

Possibly related to #75?

Given a node like:

THING
{
  transform = airlock
  transform = airlockTopB
}

and a patch targeting that node,

-transform,-1 = delete does nothing
-transform[airlockTopB] = delete deletes all of them
-transform,1 = delete seems to work, but of course it's super fragile

@Lisias
Copy link

Lisias commented Mar 11, 2024

Negative indexes are not implemented. Please read the Forum about (or at very least, the documentation).

-transform[airlockTopB] is a silent syntax error.

What you want to accomplish may be possible using MM_PATCH_LOOP, something like

@THING:HAS[#transform[airlockTopB]]
{
    <something to rule out values different of airlockTopB, and delete it when found>
    MM_PATCH_LOOP { }
}

MM patches are not a programming language. They are a convoluted FSM declaring notation.

@JonnyOThan
Copy link
Author

JonnyOThan commented Mar 11, 2024

Negative indexes are not implemented. Please read the Forum about (or at very least, the documentation).

Yes, the documentation says:

Delete - ! or -
Delete a node or value...For nodes, all options are available to select the node, including indexes. ...For values, again all options available.

(,<index-or-*>)? : Index to select particular match zero-based. 0 is the first node, -1 is the last node.

And obviously deleting ALL keys when trying to select by value is a bug.

@Lisias
Copy link

Lisias commented Mar 16, 2024

And obviously deleting ALL keys when trying to select by value is a bug.

Yes and no.

There's nothing in the documentation saying that -transform[airlockTopB] = delete should delete just one with the value airlockTopB. This syntax just doesn't exists.

What I think it's happening is the code finds the first [ and automatically thinks it's parsing an ARRAY, doesn't understand a thing, and the code degenerates into "everything you find".

Check https://github.com/sarbian/ModuleManager/wiki/Module-Manager-Handbook#arrays

The bug is not -transform[airlockTopB] = delete deleting all instances, the bug is MM not raising an error on an evident syntax error and just ignoring the problem, degenerating the declaration to -transform,*[0] = delete implicitly.

Semantic matters.

@JonnyOThan
Copy link
Author

Ah sure I did learn on re-reading the documentation that deleting a key always deletes all of them unlike nodes. So it seems like it's silently ignoring the value selector. Although again, the documentation says "all options available" but that appears to not actually be correct.

@Lisias
Copy link

Lisias commented Mar 16, 2024

The documentation sucks, no doubt.

@Lisias
Copy link

Lisias commented Mar 17, 2024

Since I opened my big mouth here, I need to clarify: there're code to handle negative indexes - but for some nodes only, and never for some values.

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

No branches or pull requests

2 participants