-
Notifications
You must be signed in to change notification settings - Fork 86
(PDOC-122) Properly parse newfunction
calls with newlines
#110
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
When `newfunction` is separated from the Puppet::Parser::Functions module name by a newline, YARD ignores the namespace and uses `newfunction` as the source of the first statement. Prior to this commit, strings didn't recognize this case, and 3.x functions written in this way were not parsed as functions. This commit updates the ruby function handler to identify and properly parse 3.x functions that include a newline between the Puppet::Parser::Function namespace and the newfunction method call.
/cc @peterhuene (I'm only picking on you because Jesse is out on vacation :)) Note also that this weird newline business is actually how we still document writing 3.x functions: https://docs.puppet.com/guides/custom_functions.html#first-function-----small-steps |
return unless statement.count > 1 | ||
module_name = statement[0].source | ||
return unless module_name == 'Puppet::Functions' || module_name == 'Puppet::Parser::Functions' | ||
return unless module_name == 'Puppet::Functions' || module_name == 'Puppet::Parser::Functions' || module_name == 'newfunction' |
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.
Naming is hard. If statement[0]
is newfunction
, it's not really a module name. Worth renaming?
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 think this is fine.
return unless statement.count > 1 | ||
module_name = statement[0].source | ||
return unless module_name == 'Puppet::Functions' || module_name == 'Puppet::Parser::Functions' | ||
return unless module_name == 'Puppet::Functions' || module_name == 'Puppet::Parser::Functions' || module_name == 'newfunction' |
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 think this is fine.
We should open a ticket to update the documentation to rid ourselves of that bad style. |
expect(tags[0].text).to eq('public') | ||
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.
yay. looks good to me. no need for acceptance
When
newfunction
is separated from the Puppet::Parser::Functions module name by anewline, YARD ignores the namespace and uses
newfunction
as the source of thefirst statement.
Prior to this commit, strings didn't recognize this case, and 3.x functions written
in this way were not parsed as functions. This commit updates the ruby function handler
to identify and properly parse 3.x functions that include a newline between the
Puppet::Parser::Function namespace and the newfunction method call.