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

Auto detect Control Repo automatically so Go to Definition works #636

Closed
JohnEricson opened this issue May 6, 2020 · 8 comments
Closed
Labels

Comments

@JohnEricson
Copy link

Summary of the new feature

Make Go to Definition work better out of the box with default settings by detecting if the workspace directory is a control repo directory.
I had to add this config in the workspace settings.json file to make this work:

"puppet.editorService.puppet.modulePath": "C:/Users/John/Documents/Git/puppet/code/modules",

Since my Git repo is C:/Users/John/Documents/Git/puppet/code it could detect if there is a modules directory in it and thus set this as the default value.

This would also make this work for other users that also uses the same workspace settings.json file. Since my coworkers use other paths to the same checked out git repo and other operating systems I can't hardcode this setting to an absolute path.

An alternative solution to solve this is to allow the setting to include VS Code's predefined variables found here https://code.visualstudio.com/docs/editor/variables-reference#_predefined-variables-examples such as ${workspaceFolder}.
Then the setting could be configured like this:

"puppet.editorService.puppet.modulePath": "${workspaceFolder}/modules",

And it would work for me and my coworkers on both Windows and Linux. But if possible auto detecting this is the absolute best solution since then it works just out of the box.

@genebean
Copy link

genebean commented May 6, 2020

Looking for environment.conf and parsing modulepath = modules:site:dist:$basemodulepath so that, in my case, modules, site, and dist are looked at might be even better

@glennsarti
Copy link
Contributor

glennsarti commented May 7, 2020

Hi @JohnEricson

The extension should already detect if the folder opened is part of a control repository, so you shouldn't need to add

The rules to detect are:

  1. You must be opening a Folder (aka workspace) in VSCode, not a single file
  2. You must be opening a Folder either at the root of, or inside of, the control repo. Not outside of it.
  3. The control repo must contain an environment.conf
  4. The control repo must NOT contain a metadata,json file. If you have both, the extension will assume it's inside a module (not a control repo)

Now there are some cases where using that setting is needed. For example, I know that @genebean has a folder containing all of the download modules from Onceover, and he needs to add the Onceover path to get the intellisense.

Some code references:

https://github.com/puppetlabs/puppet-editor-services/blob/master/lib/puppet-languageserver/document_store.rb#L105-L132

https://github.com/puppetlabs/puppet-editor-services/blob/master/spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/puppet-languageserver-sidecar_spec.rb#L226-L249

@JohnEricson
Copy link
Author

3. The control repo must contain an environment.conf

This is most likely why it isn't working for me. Our control repo is missing this file.

@rb83
Copy link

rb83 commented Dec 8, 2020

Well, for me all conditions are fulfilled, but no dice going to Symbols. Even setting puppet.editorService.puppet.modulePath doesn't help.

(In Sublime this works, strangely enough).

@puppetlabs puppetlabs deleted a comment from github-actions bot Jan 30, 2021
@jpogran
Copy link
Contributor

jpogran commented Jan 30, 2021

All, apologies for the elapsed time but I am back to working on the extension full time.

@JohnEricson I have lingua-pupuli/docs#35 open to add more documentation around this feature. I admit to not knowing alot about control repo strucutre. According to https://puppet.com/docs/pe/2019.8/control_repo.html, there should be an environment.conf in a control repo. Is there a reason it is not there? If there is a pattern/practice we're not aware of we might be able to accommodate it if we understand it better. If not, I will likely close this issue and ensure the docs are updated.

@rb83 Could you open a new issue with the versions being used? We can troubleshoot you separately, I missed your bug report because it was attached to a feature request, my apologies.

@jpogran jpogran changed the title Auto detect if workspace is a control repo and set puppet.editorService.puppet.modulePath automatically so function Go to Definition works Auto detect Control Repo automatically so Go to Definition works Feb 2, 2021
@rb83
Copy link

rb83 commented Feb 12, 2021

@jpogran I'm not sure wether it's required anymore. I've noticed that when I save a workspace and open this with vscode (as opposed to code <repofolder> in Terminal, symbols start working for me. So maybe we just need to amend rule 1 listed by @glennsarti

@jeffbyrnes
Copy link

Like @rb83, we have an environment.conf but VSCode doesn’t figure things out & show me workspace-wide symbols via ⌘-shift-r (macOS), even if I set puppet.editorService.puppet.modulePath explicitly.

@h4l
Copy link

h4l commented Dec 31, 2021

It would be useful if the rules or control repo detection @glennsarti mentions were in the documentation for the extension. The doc page on control repos reads like the feature isn't really implemented, and doesn't explain what a user can expect to work.

Code navigation wasn't working in my control repo. In my case I had a metadata.json file in my control repo in order to hack around the Puppet PDK not supporting control repos unless they have a fake metadata.json. So these two issues conflict with each other.

Resolving that also wasn't enough though, as the language server turned out to be crashing on startup, but the Puppet VSCode extension didn't report that anything was wrong. By following the instructions to enable debug logging I was able to work out that the lang server was crashing (and reported the issue here). After fixing the doc comment that triggered that issue I have code navigation appearing to work as expected.

@jordanbreen28 jordanbreen28 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants