-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fixes #7195: Canonify in ncf doesn't work like cfengine does #237
Fixes #7195: Canonify in ncf doesn't work like cfengine does #237
Conversation
# to match cfengine behaviour we need to treat utf8 as if it was ascii (see #7195) | ||
# python2 uses iso strings here, but python3 uses utf8 | ||
if sys.version_info[0] != 2: | ||
string = string.encode("utf-8").decode("iso-8859-1") |
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.
isn't the iso-8859-1 dependant on the machine locale, or user input ?
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.
this looks really dodgy to me to convert to ise-8859-1
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.
It is not dependent on the locale.
The "utf-8" is dependent on how ncf builder processes strings. And i'm pretty sure it is always utf8.
The "iso-8859-1" could be any 1 byte encoding, it is here to make sure python3 treats it as a string and not byte array so that we can apply a regex on it.
where are the unit tests ? |
3cac5a8
to
c36a3c1
Compare
PR updated |
c36a3c1
to
cccdf22
Compare
PR updated |
@peckpeck can you rebase your PR please ? :) |
cccdf22
to
38bc4da
Compare
PR updated |
…t_work_like_cfengine_does Fixes #7195: Canonify in ncf doesn't work like cfengine does
https://www.rudder-project.org/redmine/issues/7195