(#240) Fix output of default values that are expressions #315
Conversation
|
I was going to let this sit until I heard back about PUP-11632, but I figured that this was an improvement even with the bug. |
danielparks
force-pushed
the
fix_default_expressions
branch
from
a64a61d to
4cd1b15
Compare
|
I like this change. I'm just trying to weigh up the behaviour change. Does it put users in a better position? Or would this only occur some of the time? |
Previously, parameters with a default value that was an expression were
not outputted into the documentation correctly. For example,
Integer $param = 1 + 1,
Would be shown in the documentation to have a default value of “+”.
This switches to using `extract_tree_text` instead of `extract_text` to
get the text representation of the parsed Puppet code.
This also gets rid of the dependency on
`Puppet::Pops::Adapters::SourcePosAdapter`, which was [deprecated in
2017](puppetlabs/puppet@68498ad).
Unfortunately, it appears that there is a bug in Puppet ([PUP-11632][])
that sometimes returns the incorrect default value. This adds two
skipped tests that demonstrate the issue.
[PUP-11632]: https://tickets.puppetlabs.com/browse/PUP-11632
danielparks
force-pushed
the
fix_default_expressions
branch
from
4cd1b15 to
2088d38
Compare
|
You’re asking how often the PUP-11632 bug occurs and gives a bad default value? It seems fairly rare. So far I’ve only found two cases, which are both documented in the bug and in the unit tests attached to this PR. It seems to have much less impact than the bug this PR fixes. As is, you often get what appears to be an incomprehensible fragment of the default value, e.g. With this PR you get a few munged values, but so far it’s only been a missing final character, e.g. |
|
@danielparks thanks for the explanation .. it's much clearer now. I'm happy to get this merged! |
Previously, parameters with a default value that was an expression were not outputted into the documentation correctly. For example,
Would be shown in the documentation to have a default value of “+”.
This switches to using
extract_tree_textinstead ofextract_textto get the text representation of the parsed Puppet code.This also gets rid of the dependency on
Puppet::Pops::Adapters::SourcePosAdapter, which was deprecated in 2017.Unfortunately, it appears that there is a bug in Puppet (PUP-11632) that sometimes returns the incorrect default value. This adds two skipped tests that demonstrate the issue.