Closed orichters closed 1 month ago
@orichters for such a critical function it is really complex to have all relevant use cases in mind. That said, your changes look sensible to me. Luckily, we already have some tests capturing the main use cases of the function. Please also add test cases reflecting the new aspects your PR tries to cover (e.g. a test making sure that a comment is not messed up anymore)
I will add some tests, although I am quite confident it works because of this new test in remindmodel that shows that the 1831 messy lines of main.gms
are not changed at all by running manipulateConfig with the default configuration.
The more crucial question to me seems to be: is there any possibility where it might be intended to manipulate a line such as
Parameter yourswitch = yourvalue
or something like that, where yourswitch
is not at the beginning of the line. I don't see any reason to do that, but I'm not sure enough…
to my understanding the case in which the line starts with "parameter" or similar is already covered by the search&replace pattern2 which you did not change, right?
to my understanding the case in which the line starts with "parameter" or similar is already covered by the search&replace pattern2 which you did not change, right?
Yes, scalars or parameters are not affected by this change.
main.gms
file (with the default settings read from that file) leads to the substitutions below. Any occurence of a switch name followed by=
is substituted, leading to completely unreadable and misleading comments, and also leading to model fails.variable = value
constructions that show up at the beginning of the line, not anywhere..gms
file, this is not going into this part of theif
-chain.