| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
| |
When we introduced verification of options, we forgot to handle the case that
global options from the Puppet settings system could be passed to the face.
This, in turn, means that the system would fail if you used any of those.
This remediates that, and now these work as expected.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
| |
Rewrite the process of validating and updating the options to fully reflect
the contract - we fail if there are unknown options passed, report nicely
errors of duplicate names, pass only the canonical names to the action code,
and generally enforce nicely.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than having multiple, separate operations that modify and validate the
arguments to an action, a single pass makes sense. This also means less walks
across the set of data, and a few less expensive method calls in Ruby.
Additionally, we work on a duplicate of the arguments hash rather than
directly modifying the original. Because everything we do is at the top level
key/value mapping, this is sufficient to isolate the original.
While mostly theoretical, we now don't mutilate the hash passed in, so the
user won't get nastily surprised by the fact that we could have done so.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
When we define an action on an older version of a Face, we must be sure to
directly load the core of that version, not just define it with the external
Action(s) that it had.
Otherwise we break our contract, which is that any core Actions for a specific
version will be available to your external Action for as long as we support
that core version.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we first touch a Face, we load all the available Actions from disk.
Given they define themselves against a specific version of a Face, they are
automatically available tied to the correct version; this makes it trivially
possible to locate those on demand and return them.
Now, we have the ability to find and, consequently, invoke Actions on older
versions of Faces. We don't load enough context, though: the older face will
only have external Actions defined, not anything core.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
As part of moving to load actions first, and their associated face, when
invoked from the command line, it makes sense to push the logic for finding
the action and face down into the Puppet::Face implementation.
This means that we can change the logic there without needing to update the
public part of the CLI implementation, and that any further facades can use
the same, correct, logic to locate the action for the face.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
Given the inheritance model for actions, we are sometimes going to need to set
them to 'default' at runtime, rather than during their static declaration.
Add tests to verify that this works correctly, and update the code to ensure
that happens. This gives up caching of the default action, but this should be
an extremely rare operation - pretty much only CLI invocation, really.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
This implement support for options with default values, allowing faces to set
those values when not invoked. This can eliminate substantial duplicate code
from actions, especially when there are face-level options in use.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
| |
This introduces a class representing a semantic version, and
implementing a few of the most common uses of them: validation,
comparison, and finding the greatest available version matching
a range. This refactoring also allows us to easily expand our
matching of version ranges in the future, which is a big plus.
Reviewed-By: Daniel Pittman
|
| |
|
|
|
|
|
|
| |
The problem was caused by the fact that the
options method returns a list of options that
treated the aliases as seperate options.
The fix is to only maintain a list of options
and not add all aliases to the options list.
|
| |
|
|
|
|
|
| |
2.7.x"
This reverts commit b7ee0258ab40478329c20177eda9b250f27ede18, reversing
changes made to 8fe2e555ac3d57f5b6503ffe1a5466db8d6e190a.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
puppet help was reprinting every option once
for every alias that is had.
This fix involves only storing the option.name
in the @options instance var for both face and
actions options. The @options_hash still
maintains the list of options and aliases as its
keys.
Reviewed-by: Daniel Pittman (puppet-dev list)
|
| |
|
|
|
|
|
|
|
|
| |
Rdoc wouldn't parse lib/puppet/interface/options.rb
The offending code has been removed.
This was causing issues for users wishing to upgrade puppet,
via gem or puppet.
Signed-off-by: Michael Stahnke <stahnma@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We accidentally omitted whitespace between multiple options while building the
synopsis. This fixes that, by introducing a breakable space in the right
location.
Additionally, we extract the code that was 99 percent identical from the face
and action synopsis generators, push it down into the documentation module,
and then invoke it from both places.
This eliminates the duplicate code, allowing me to fix that bug once and have
it apply to both parts of the code; this is pretty much assured to be true
any time we change the synopsis generation.
Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
We earlier moved to duplicating Action objects in the Faces subsystem to
ensure they had the correct binding context during execution and introspection.
This was correct, but introduced a bug where we would report both the parent
and child binding as separate entries with duplicate names, in the list of
actions.
This flowed on to the help output, where it would cause every inherited action
to be listed twice: once on the parent, once on the child. (This was actually
worse if the inheritance was deeper: we would duplicate once for every level
between the instance and the origin of the action.)
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
ticket/2.7rc/maint-doc_changes_without_failures
* ticket/2.7rc/maint-faces_docs_spec_fixes:
maint: Fix order dependent spec failure for face indirection
(#7690) Don't blow up when listing terminuses available for faces
maint: Dedup the loadpath so we don't have to walk it multiple times
Maint: Fix ellipses for short descriptions
Resolved Conflicts:
lib/puppet/interface/documentation.rb
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, we were adding ellipsis to any short_description, which was
misleading; they were only necessary when we were truncating in the _middle_ of
a paragraph.
This commit changes the behavior, and updates the tests to show when we expect
ellipsis.
Paired-with: Matt Robinson <matt@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| | |
Auto-generated short descriptions cut off at five lines with no
indication that they are truncated.
This commit adds ellipsis in brackets to indicate incompleteness.
|
| |/
|
|
|
|
|
|
|
|
| |
Since some actions take arguments and some do not, action synopses were
incomplete and ambiguous.
This commit adds a method for explicitly declaring what argument(s) an
action takes, and places the arguments at the appropriate spot in the
action's synopsis. By convention, individual arguments should be wrapped
in angle brackets.
|
| |
|
|
|
|
|
|
| |
Ruby 1.9 is strict about argument arity for methods that are
metaprogrammatically defined. A ton of specs that were setting up
when_invoked didn't pass options even though they should have been.
Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
instance_methods in Ruby 1.8.7 returns an array of strings, but returns
an array of symbols in 1.9.2. This manifested itself when running the
tests because in 1.9.2 we were trying to call sub on a sybmol. The
original proposed solution was to monkey patch symbols to have a sub
method, but this didn't deal with the real issue of need to check
whether a method was defined, and actually made it worse.
Turns out that checking for the presence of a method in an array that
may contain symbols and may contain strings is better done by just
calling method_defined? instead.
This patch addresses all the places ack turned up the code doing this
include? check instead of directly calling method_defined?.
Thanks to Alex Sharp ajsharp@gmail.com for pointing out the Ruby 1.9
problems and working toward a solution.
Reviewed-by: Nick Lewis <nick@puppetlabs.com>
|
| |
|
|
|
|
|
| |
Since we never shipped this in a real release, we don't need to maintain
compatibility. So, remove it entirely from the codebase.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
Where we need special support for :for_humans as an alias for :console, call
it out in comments. This makes it clear to someone who wonders why what the
actual underlying purpose of the whole thing is.
Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
We refer to rendering formats pretty consistently as `json`, `yaml`, `s`, and
so forth; unqualified names.
On the other hand, we refer to the rendering hooks *mostly* as `to_*`, except
the `:for_humans` and `:json` formats. Which is kind of confusing because of
the internal inconsistency, and because it doesn't match the public name.
Fix the code to resolve both, so the `to_*` format still works, but we mostly
expect to use the `*` version, to match public expectation.
|
| |
|
|
|
|
| |
Turns out that String#each_line on Ruby 1.8.5 only takes a block, and will not
return an enumerable. Rewrite to use split(/\n/) which has the same effect
but works on all our platforms.
|
| |
|
|
|
|
|
|
| |
We want to be able to document the data returned from an action, since this is
one of the most critical parts of the API for Ruby developers. This adds a
multiline documentation block that captures that.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
This is unnecessary, and only turned up because Matz &c. impose their taste on
the rest of the world through the Emacs Ruby mode. Since people are starting
to clone that, and it doesn't add value, eliminate it everywhere.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
| |
When we hit a syntax error in any face, a whole bunch of unrelated face things
would blow up in horrible ways. Stack traces for all...
Now, instead, we catch that fault but specifically only in the face file
and report it through our error logs, then quietly ignore the face.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
We used to let actions be declared without the `when_invoked` block, which was
usually a sign of either someone writing their method code direct in action
declaration, or someone forgetting to add their code at all.
This was just let silently by: the error only showed up when you finally tried
to invoke the action, and a NoMethod error was raised by the face.
...except for our own testing. We took advantage of this a whole pile of
times in there; fixing the original UI issue means fixing all those too.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We now strip whitespace in face (and related) documentation in two places:
We strip any trailing whitespace on each line, just because.
We strip any leading indent, but not all leading whitespace, from the text.
That is, we strip the *minimum* amount of whitespace that we can take from
every line in the documentation without changing the overall content.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |\ \
| |/
|/| |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`before_action` decorations should always resolve in resolution order
from most general (inherited from furthest away) to most specific
(declared on the instance), and should always execute Face-level
option decorations before action-level option decorations.
`after_action` decorations should execute in the opposite order.
Reviewed-By: Daniel Pittman
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
From the command line, and other facades, it is fairly difficult to call an
action with two aliases of the same option in the invocation, but from the
Ruby API it would be relatively easy to achieve.
This is now checked, and raises an error, so that we don't accidentally have
strange or unusual behaviour coming out of the whole system.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We had a problem, previously, in the generic translation of command line
arguments to Ruby method calls: we could mistake the options, added by the CLI
wrapper, for a positional argument to the action method.
This was caused by a combination of factors, but primarily that the wrapper
methods for actions are designed to present a friendly, helpful Ruby API for
internal use. Consequently, they have a default value if you don't wish to
pass options.
Unfortunately, this meant that the options that the CLI *always* passed could
be treated as a positional argument instead, and the default set of options
added to the back of the call.
To resolve this we now check the number of positional arguments in the CLI
wrapper, and raise an exception if they are mismatched. This makes the
generic CLI handling do the right thing in adapting the command line
arguments to the Ruby API.
(As an aside, we would have had a similar-but-different failure mode if we
type-checked positional arguments: these calls would have failed with an
invalid argument validation error.)
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |\ \
| |/
|/| |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We now run all the faces, and their actions, as well as global help through
the wringer in this test: this way we can be confident that we have, at least,
the ability to generate the help without a user-visible failure.
We also check that we have set copyright and license terms in our own faces.
Theoretically this might fail if the end user has extra faces on LOAD_PATH,
but my hope is that we won't hit that...
|
| | |
| |
| |
| |
| |
| |
| | |
This extends the last of the documentation support, down into options, so they
can be described as expected. In the process we split out the modular docs
API into a full and short version options only want short docs, but the
behaviours are identical to the full version.
|
| | |
| |
| |
| |
| |
| | |
Given that we have identical documentation behaviour in the face and action
code, it should properly be written once. So, move it into a module, extend
the other classes with it, and have done.
|
| | |
| |
| |
| |
| |
| | |
This allows users to write before_action advice that does basic
option validation very easily.
Reviewed-By: Daniel Pittman
|
| |/
|
|
|
|
|
|
| |
This change permits users to call functions with a reference to
`self` that can augment the in-progress action declaration, which can
be helpful in some more involved cases.
Reviewed-By: Max Martin
Reviewed-By: Daniel Pittman
|
| |
|
|
|
|
|
|
| |
A whole pile of spec files for faces were not pulling in the regular
spec_helper, or the puppet/face library before they used it. This worked fine
by coincidence when they ran together, but blew up if run separately.
Reviewed-By: Jesse Wolf <jesse@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
"Invisible glob", or "prefix", version matching means that when you specify a
version string to use you can specify as little as one version number out of
the semantic versioning spec.
Matching is done on the prefix; an omitted number is treated as "anything" in
that slot, and we return the highest matching versioned face by that spec.
For example, given the set of versions: 1.0.0, 1.0.1, 1.1.0, 1.1.1, 2.0.0
The following would be matched:
input matched
1 1.1.1
1.0 1.0.1
1.0.1 1.0.1
1.0.2 fail - no match
1.1 1.1.1
1.1.1 1.1.1
1.2 fail - no match
|
| |
|
|
|
|
|
|
|
|
|
|
| |
We used to treat anything with a top level key in the faces hash as a valid
face; it makes more sense to filter that only to things that have at least one
implementation.
Previously we had to be super-careful not to accidentally touch the top level
for an invalid face, which set us up for future failure when someone wasn't
careful enough; now we can cope with that.
Paired-With: Max Martin <max@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We previously used *args to collect all arguments to the action when_invoked
block, then tried vaguely to massage some little bits of them into the right
shape.
Methods defined with blocks, in Ruby 1.8, also have some fun behaviours. The
most special is that if you pass more than one argument to a block defined
with only one Ruby will automatically coerce the arguments into an array – and
this is preserved when it is bound to a method.
This led to routine situations where we would pass the wrong number of
arguments to the block because, say, the user gave an extra argument on the
command line.
Instead of failing this would transmogrify the arguments in counterintuitive
ways, and end up with horrible stack traces when that interacted badly with
the code as written.
Now, instead, we work out the right argument format based on the arguments
that the when_invoked block takes. This gives much better (albeit perhaps not
so user friendly) behaviour at the interface level. Which is, at least,
consistent with other Ruby API.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
The comment discussing the purpose of the wrapper and related details rightly
belongs outside the method; move it there so it doesn't perturb the functional
changes that follow.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |\
| |
| |
| | |
Fix the conflicts over changes in my previous commit.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We now find, and call, the appropriate rendering hooks on actions during the
rendering phase. This allows the user to intercept and replace the result
object that passes through the rest of the rendering system on the fly.
Example usage:
action :foo do
when_rendering :pson do |result|
{ :whatever => result[a],
:foobar => result[b],
}
end
end
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We no longer establish the rendering mode in the actions; they just default to
"nothing", and let that flow on out to the application layer. That lets the
facade we put before the face determine the default behaviour.
This is mostly a no-op down in the CLI side, but it makes it much easier to
integrate into MCollective, HTTP-API, and for other non-CLI users of Faces.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Now we want to support action-based rendering, it is super-hard to define the
semantics around defaulting where things are unspecified: the execution
context (CLI, HTTP, etc) vs the face, vs the action all have different
semantics.
Without solving the problem of how we express all that context and those
semantics down in the action, especially one written by a third party, this
just becomes a box of counter-intuitive and annoying semantics and edge-cases.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
