-
Notifications
You must be signed in to change notification settings - Fork 8
PLT-294 Added slash command to check used version of CSH #28
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
Conversation
"require version csh" is useful in .csh scripts (init or not), where it can tell CSH to: exit, error or warn when a version constraint fails.
So it's now possible to require version ==2.5.*
They are failing of course. So I guess I have to fix that.
…or compare_version() Also expanded the unit tests quite a bit.
|
Alright, this PR should be ready now (with wildcard support, unit tests and all). All we may want to do is add some more fail-cases for compare_version(). |
|
Just noticed that Valgrind compains an initialized value being passed to sscanf() here: Line 170 in ae87ba1
Either way it doesn't make much sense since both constraint and constraint_version are very much zero initialized: Lines 106 to 108 in ae87ba1
|
jeanbaptistelab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…csh" slash command.
|
Let us get it on master |
We also no longer use a mutable copy of the version string, so we allow much larger versions.
With the recently added feature of the __FILE_DIR__ environment variable, it becomes apparent that CSH versions from before this commit will not work with scripts requiring the variable to be present.
This PR introduces a feature to help with such scenarios.
The new slash command: "require version csh" is useful in .csh scripts (init or not), where it can tell CSH to: exit, error or warn when a version constraint fails.
At the time of writing, the version constraint argument supports the typical comparisons: "==", "!=", ">=", "<=", ">" and "<".
But I have yet to add support for wildcard constraints, which would be useful for the patch version.
The name of the command is a bit clunky, as the intention is to also allow a command to check APM versions in the future.
Of course, if there are better suggestions for the name before merging, I would be happy to hear them.
The same can be said for the current error-level names: "fatal", "error" and "warn", where we may want to rename "fatal" to quit, so the all represent the resulting action.
I would like for the first letter of the error-level words to be different, as we currently allow single letters for short notation, of course this can also be debated.