Skip to content
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

code completion for notify should not have ensure #67

Closed
glennsarti opened this issue Oct 25, 2018 · 1 comment · Fixed by #73
Closed

code completion for notify should not have ensure #67

glennsarti opened this issue Oct 25, 2018 · 1 comment · Fixed by #73
Labels
bug Something isn't working Language Server
Milestone

Comments

@glennsarti
Copy link
Contributor

From:
puppetlabs/puppet-editor-syntax#5

Originally created by @TJM


What Versions are you running?

Editor Name and Version: VSCode 1.26.1
Theme: (dark?)
Document: test.pp

What You Are Seeing?

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: no parameter named 'ensure' (file: /etc/puppetlabs/code/environments/production/site/profile/manifests/test.pp, line: 106) on Notify[testing]

What is Expected?

the message I tried to notify out

How Did You Get This To Happen? (Steps to Reproduce)

in a puppet file type "notify" and hit "tab" when the code completion dialog comes up... resulting output is:

notify { 'title':
  ensure => 'present'
}

The "ensure" parameter is not valid.

@glennsarti
Copy link
Contributor Author

glennsarti commented Oct 25, 2018

This is indeed a bug in PES.

https://github.com/lingua-pupuli/puppet-editor-services/blob/9b19f6b6b576b1d0bdc7d56ff8971f60b171862a/lib/puppet-languageserver/manifest/completion_provider.rb#L175

This naively adds ensure. We have information in the object cache now to know which resource properties are required, so we could just populate those now.

@glennsarti glennsarti added bug Something isn't working Language Server labels Oct 25, 2018
@glennsarti glennsarti added this to the 0.16.0 milestone Oct 25, 2018
glennsarti added a commit to glennsarti/puppet-editor-services that referenced this issue Nov 9, 2018
Previously when a completion resolution request was received for a resource it
was hardcoded to return the name and ensure property.  However not all resources
are ensurable, and some resources have other required properties or parameters.
This commit changes the resource completion to interrogate the Puppet Type and
then generate a more appropriate code snippet;

* The sidecar was extended to also track the isnamevar? value for resource
  properties and parameters. This was needed by the completion resolver as
  namevars should not appear in the code snippet as they come from the resource
  name.

  This also required tests to changed for this new property in the sidecar
  protocol.

* The resource_type completion request was modified to dynamically generate the
  snippet for resources;
  - If the resource is ensurable, then the ensure parameter now appears the
    first in the list, as per puppet-lint
  - Properties/Parameters which are marked as required, are included in the code
    snippet, except for those marked as a namevars.  They implicitly come from
    the resource name.
  - The resource property/param names are sorted in alphabetical order (except
    for ensure) as per puppet-lint
  - The hash rockets of the generated snippet are now aligned as per puppet-lint

* Added tests for the new rsource completion style

* Fixed a minor bug which really only shoed itself during testing and wouldn't
  normally be an issue in normal operation.  The object being passed into the
  `resolve` was being mutated inside the method.  However the intended behaviour
  of the method was output a new object and modify the input object in any way.
  This commit instead clones the input object very early and then uses the clone
  internally.
jpogran pushed a commit that referenced this issue Nov 13, 2018
@glennsarti glennsarti added this to To do in 0.14.0 Marketplace Release via automation Nov 15, 2018
@jpogran jpogran moved this from To do to Done in 0.14.0 Marketplace Release Nov 15, 2018
@jpogran jpogran removed this from Done in 0.14.0 Marketplace Release Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Language Server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant