-
Notifications
You must be signed in to change notification settings - Fork 98
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
Sanitize library names before running liblist command #1673
Conversation
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.
I'd consider this a high impact change since it is used by a lot of our extension (and others!).
The tests are passing and things are working as normal.
Other than the unused import, the code and functionality works good.
Optionally, if you felt inclined, I'd love to see a new test case to show that it works with libraries that start with #
.
@worksofliam I'm currently working on some tests - which revealed some defects with the handling of '$'! EDIT: The '$' defect seems to be an old error - one can add '$LIBRARY' to the library list in the current version of C4i! |
@worksofliam I fixed errors for sanitizing names with '$' and validating library list with sanitized names - and finally added some tests to verify library name sanitizing. Please have another go... |
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.
Looking good @chrjorgensen! Thanks for your work here.
Changes
This PR will fix the error described in issue #1672.
It will also fix errors in other library list commands:
#
#
when changing the library listChecklist