I think the current script was fine. But since you explicitly asked for a review ...
I've described my changes in the commit message (also pasted below).
As before: I did VERY LITTLE tests on my changes. And feel free to disagree with the changes in functionality.
Wouter
Commit message:
It's strange to allow "auto_next false" but not "auto_next true". Therefor I
generalized it to 'auto_next []'. Any Tcl boolean syntax is allowed (yes,
no, true, false, 0, 1, ..), not specifying the (optional) parameter is the same
as 'true'. I agree that in practice, in interactive mode, the user will never
type "auto_next true". But it could be useful when called from another script.
I did not yet adapt the help text for this stuff, but maybe that's also not
needed?
Personally I don't like errors on stuff like:
vgm_rev auto_next false
ERROR: Auto_next is not active, can't disable it.
Maybe I'm using this in the context of a script and I want to make sure
auto_next is disabled, but I don't know what the current status is. In other
words enabling/disabling when the thing is already enabled/disabled should do
nothing instead of generating an error.
Hi,
I think the current script was fine. But since you explicitly asked for a review ... I've described my changes in the commit message (also pasted below).
As before: I did VERY LITTLE tests on my changes. And feel free to disagree with the changes in functionality.
Wouter
Commit message:
It's strange to allow "auto_next false" but not "auto_next true". Therefor I generalized it to 'auto_next []'. Any Tcl boolean syntax is allowed (yes,
no, true, false, 0, 1, ..), not specifying the (optional) parameter is the same
as 'true'. I agree that in practice, in interactive mode, the user will never
type "auto_next true". But it could be useful when called from another script.
I did not yet adapt the help text for this stuff, but maybe that's also not
needed?
Personally I don't like errors on stuff like: