-
Notifications
You must be signed in to change notification settings - Fork 583
add suffix() to go along with prefix() #138
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
# | ||
# suffix.rb | ||
# | ||
|
||
module Puppet::Parser::Functions | ||
newfunction(:suffix, :type => :rvalue, :doc => <<-EOS | ||
This function applies a suffix to all elements in an array. | ||
|
||
*Examples:* | ||
|
||
suffix(['a','b','c'], 'p') | ||
|
||
Will return: ['ap','bp','cp'] | ||
EOS | ||
) do |arguments| | ||
|
||
# Technically we support two arguments but only first is mandatory ... | ||
raise(Puppet::ParseError, "suffix(): Wrong number of arguments " + | ||
"given (#{arguments.size} for 1)") if arguments.size < 1 | ||
|
||
array = arguments[0] | ||
|
||
unless array.is_a?(Array) | ||
raise(Puppet::ParseError, 'suffix(): Requires array to work with') | ||
end | ||
|
||
suffix = arguments[1] if arguments[1] | ||
|
||
if suffix | ||
unless suffix.is_a?(String) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to have a test to ensure that the suffix method fails if the second argument is a string. Also, what happens if the suffixed value is an integer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "suffix" function is a carbon copy of stdlib's existing "prefix" function, with the only change being to append rather than prepend (which is a single line change, aside from s/prefix/suffix/g). So I'm not sure how to respond to these comments, since they are actually comments about the already-shipping "prefix" function, authored by "Krzysztof Wilczynski" (perhaps on github somewhere?). Given that "prefix" is considered good as-is, accepting 'suffix' and then filing an issue against puppetlabs-stdlib to address these comments for both prefix and suffix would ensure that your comments are addressed without unnecessarily delaying inclusion of a useful function. (This pull request was submitted at the urging of our trainer, so you can also come find me in the PDX classroom today and tomorrow between 9a and 4p, if that's useful.) |
||
raise(Puppet::ParseError, 'suffix(): Requires string to work with') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as with array; this error message should indicate that the second argument should be a string. |
||
end | ||
end | ||
|
||
# Turn everything into string same as join would do ... | ||
result = array.collect do |i| | ||
i = i.to_s | ||
suffix ? i + suffix : i | ||
end | ||
|
||
return result | ||
end | ||
end | ||
|
||
# vim: set ts=2 sw=2 et : |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#! /usr/bin/env ruby -S rspec | ||
require 'spec_helper' | ||
|
||
describe "the suffix function" do | ||
let(:scope) { PuppetlabsSpec::PuppetInternals.scope } | ||
|
||
it "should exist" do | ||
Puppet::Parser::Functions.function("suffix").should == "function_suffix" | ||
end | ||
|
||
it "should raise a ParseError if there is less than 1 arguments" do | ||
lambda { scope.function_suffix([]) }.should( raise_error(Puppet::ParseError)) | ||
end | ||
|
||
it "should return a suffixed array" do | ||
result = scope.function_suffix([['a','b','c'], 'p']) | ||
result.should(eq(['ap','bp','cp'])) | ||
end | ||
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.
Since Puppet functions use positional arguments, it would be good to indicate that the first value should be an array, instead of one of the values should be an array.