Skip to content

Conversation

@lelutin
Copy link
Contributor

@lelutin lelutin commented Aug 18, 2024

This is using Puppet's own DAP server in the puppet-editor-services project.

Debugging puppet is a tiny bit weird.. it mostly means debugging the catalog building that the puppet server would be operating.

It gets weirder in that to debug in a real situation you need to supply variables and facts that would be in place for the host in question.

But it can nonetheless be done, and the DAP server permits stepping through the catalog building steps through the code while examining variables and resources as they are evaluated.

This is using Puppet's own DAP server in the puppet-editor-services
project.

Debugging puppet is a tiny bit weird.. it mostly means debugging the
catalog building that the puppet server would be operating.

It gets weirder in that to debug in a real situation you need to supply
variables and facts that would be in place for the host in question.

But it can nonetheless be done, and the DAP server permits stepping
through the catalog building steps through the code while examining
variables and resources as they are evaluated.
@puremourning
Copy link
Owner

This change is Reviewable

@lelutin
Copy link
Contributor Author

lelutin commented Aug 18, 2024

hopefully I haven't missed any details for documentation.

this is a tiny improvement on the documentation for extra languages in that the model: "simple" lays out the symlink to the downloaded code correctly.

something that I saw was that the tar archive was downloaded and then extracted but never removed. but if I'm reading the code right, the same would supposedly happen with .zip files (although I don't see this happening with debugpy or vscode-bash-debug... so I'm still left scratching my head on that detail)

@lelutin
Copy link
Contributor Author

lelutin commented Aug 18, 2024

note: if this gets merged in, we should clear out the Puppet section from the wiki page https://github.com/puremourning/vimspector/wiki/Additional-Language-Support#puppet

@puremourning
Copy link
Owner

Thanks for the PR!

I guess my main question is, given that I don't personally use puppet.. what's the advantage of this vs the wiki page? Is there something that's not possible with the "limited" configuration provided by the gadget installer extension vs the builtin models?

Normally, these days at least, I figure that we only should include gadgets built-in if:

  • it's not possible to use the simple model (i.e. it needs some 'do' function to actually bootstrap it)
  • it's a very common feature lots of users will need (c, c++, python, javascript, you get the idea)
  • I personally would use it regularly or wrote the debug adapter (like TCL, in my previous job)
  • etc.

There's no hard-and-fast rule about it though, so interested in your thoughts.

Finally, comparing the additional-laguage-support page vs this implementation:

  • we no longer default cwd in the adapter configuration. intentional?
  • in the wiki page we have example vimspector config for a project, we don't have that in README.md.
  • on the wiki page, we have some notes and considerations. Do they still apply?

vimspector connects explicitly to the ipv4 localhost address. This is
meant to avoid irregularities caused by using "localhost" which may end
up being ::1 for some hosts instead. Both vimspector and all the LSP
servers need to agree on the address family being used.

IPv6 has gained some more traction in the recent years but it's still
not reached enough people so that we could instead default to ::1 for
all.
@lelutin lelutin force-pushed the add_puppet_gadget branch from a4684f5 to 4c4da62 Compare August 18, 2024 12:52
@lelutin
Copy link
Contributor Author

lelutin commented Aug 18, 2024

thanks for your feedback!

Creating custom gadgets sure is possible, but it's rather non-trivial. The choice of options that are needed to make vimspector install the right thing in the right place and with the symlinks pointing to the right directory are rather hard to get right. I unfortunately did get things wrong when I suggested the puppet section of the wiki, so in its current state the gadget is not getting installed correctly (the symlink is created as dangling).
The only reason I was able to finally get this working correctly this time was because I ended reading the plugin's python code.

So in that sense, adding more gadgets ready for users to install for local usage is a plus in terms of UX. It means that ppl can add this new gadget to g:vimspector_install_gadgets, run VimspectorInstall and than only need to bother with creating the .vimspector.json configuration.

With regards to the other considerations:

  • for the cwd, I don't think that it has an influence on the puppet-languageserver since users will possibly want to specify --modulepath as an option to puppet, but I could be mistaken. In any case it doesn't hurt if it's set so we could add it back.
  • for the example .vimspector.json: you're right! I was wondering about this detail. I didn't see any such example configs for all of the other builtin gadgets though so I wasn't sure where I should add the snippet. Maybe we could start a new section in the readme for example configuration snippets, somewhere close to the examples about remote debugging?
  • yes, the considerations near the configuration snippet for puppet in the wiki should be kept around. I also have two other points that I could add there: using --verbose in the puppet args to get more traces in the output window of what's being done by the puppet parser, and the fact that debugging files that simply declare new structures is pointless (the debugger won't do anything since it will just produce an empty catalog -- which is what the debugger is going through) and that one rather needs a test file that actually includes the code declared.

awaiting for your feedback before I do any more changes regarding the last points

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants