-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
WIP: Enum new PUP-10944 #8710
base: main
Are you sure you want to change the base?
WIP: Enum new PUP-10944 #8710
Conversation
Can one of the admins verify this patch? |
95c4b05
to
e1a9383
Compare
pick one item of an enum. A boolean or integer value is accepted and a string is returned.
Hi @nod0n, thanks for your contribution! I read through the slack conversation, about wanting something like notice(convert(['no', 'yes'], true)) If so I'm not sure adding end
dispatch :from_args do
+ param 'Array', :values
param 'Selector', :selector # selects one of the Enum's strings
optional_param 'Format', :string_format # how should the string being formatted?
end
- def from_args(selector, format = :default)
+ def from_args(values, selector, format = :default)
string = case selector
- when true then @values[1]
- when false then @values[0]
- when Integer then @values[selector]
+ when true then values[1]
+ when false then values[0]
+ when Integer then values[selector]
end
StringConverter.singleton.convert(string, format)
However that fails, because
Based on your original comments in slack:
makes me think you really do want to implement |
An ”instance of Enum” is actually a ruby string so that should work. There are never any instances of any other ”enum type” passed around. The check in new impl that of the return value’s type could be wrong for Enum - probably because Enum did not have a new before this so has gone untested for that case.
I looked briefly at String new, and as I recall it was difficult to make it support the proposed “pick string from array or enum based on value”. I could be wrong though.
- henrik
… 1 sep. 2021 kl. 01:12 skrev Josh Cooper ***@***.***>:
Hi @nod0n, thanks for your contribution! I read through the slack conversation, about wanting something like bool2str from stdlib. If I understand correctly, you want to take a boolean (or integer) as input and have it select among multiple boolean-like values (yes, no, etc), for example:
notice(convert(['no', 'yes'], true))
If so I'm not sure adding Enum.new will do that, but I may be misunderstanding. For example, I took your PR and added the following:
end
dispatch :from_args do
+ param 'Array', :values
param 'Selector', :selector # selects one of the Enum's strings
optional_param 'Format', :string_format # how should the string being formatted?
end
- def from_args(selector, format = :default)
+ def from_args(values, selector, format = :default)
string = case selector
- when true then @values[1]
- when false then @values[0]
- when Integer then @values[selector]
+ when true then values[1]
+ when false then values[0]
+ when Integer then values[selector]
end
StringConverter.singleton.convert(string, format)
However that fails, because Enum.new is supposed to return an instance of an Enum, not a String:
$ bx puppet apply -e 'notice(Enum.new(["no", "yes"], true))'
Error: Evaluation Error: Error while evaluating a Method call, Converted value from Enum.new() has wrong type, expects a match for Enum, got 'yes' (line: 1, column: 16) on node localhost
Based on your original comments in slack:
but is it also possible to set custom value for true , like with bool2str
makes me think you really do want to implement String.new(["no", "yes"], true)? Thoughts @hlindberg?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Ahh, ok I'll take a look |
when true then @values[1] | ||
when false then @values[0] | ||
when Integer then @values[selector] | ||
end |
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.
When the selector
is an Integer
we should assert that it's between 0 <= selector < @values.length
and raise ArgumentError
if it's not. Similarly, we should have an else
clause and raise if since it's not supported. We do something similar when creating floats:
raise ArgumentError, _("Illegal radix: %{radix}, expected 2, 8, 10, 16, or default") % { radix: radix }
https://tickets.puppetlabs.com/browse/PUP-10944