Skip to content

(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

Merged
merged 1 commit into from
Oct 10, 2016

Conversation

whopper
Copy link
Contributor

@whopper whopper commented Oct 10, 2016

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.

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.
@whopper
Copy link
Contributor Author

whopper commented Oct 10, 2016

/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'
Copy link
Contributor Author

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?

Copy link
Contributor

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'
Copy link
Contributor

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.

@peterhuene
Copy link
Contributor

peterhuene commented Oct 10, 2016

👍 although Ruby developers that open a module to invoke a function on it...

@peterhuene
Copy link
Contributor

We should open a ticket to update the documentation to rid ourselves of that bad style.

@peterhuene peterhuene merged commit d9c974a into puppetlabs:master Oct 10, 2016
expect(tags[0].text).to eq('public')
end
end

Copy link
Contributor

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

@chelnak chelnak added the bugfix label Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants