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

Add definitionPath property to debug configuration attributes #116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

FlorentVSTM
Copy link

This PR adds the property 'definitionPath' to the attributes of the 'gdbtarget' debug configuration. This property comes from the peripheral-inspector extension.
Adding this property promotes the use of the eclipse-cdt.peripheral-inspector extension and avoids the yellow squiggly line in the launch.json file.

@eclipse-cdt-bot
Copy link
Contributor

Can one of the admins verify this patch?

@colin-grant-work
Copy link
Contributor

@FlorentVSTM, can you say a little bit more about how the two extensions interact? How does the eclipse-cdt.peripheral-inspector consume the setting for a gdb debug configuration?

@FlorentVSTM
Copy link
Author

@FlorentVSTM, can you say a little bit more about how the two extensions interact? How does the eclipse-cdt.peripheral-inspector consume the setting for a gdb debug configuration?

Hi @colin-grant-work,

yes of course, The peripheral inspector consumes the definitionPath option from the gdb debug configuration thanks to the debug session provided by the tracker "onWillStartSession" (periperalTreeProvider.ts).
Then, in svd-resolver.ts the resolve method checks if the debug configuration of this debug session contains the definitionPath option for use.
Note that users can change the name of the definitionPath option in the extension settings, but by default, it is definitionPath.

@colin-grant-work
Copy link
Contributor

The use case sounds reasonable, but I'm not sure the approach is sound, because I'm not sure that it would make sense to document in this plugin all the data that any variant of GDB debugging might use: it should document the schema expected by the CDT-GDB Adapter, whose functionality it exposes to VSCode. On the other hand, I'm not aware of a mechanism in the VSCode API that would allow another plugin to modify the schema of the debug configuration declared here without supplying a debug configuration of its own, which also isn't ideal.

Let me take a look at the code that handles debug schemas and see if we have any options that would allow for a cleaner separation.

@FlorentVSTM
Copy link
Author

FlorentVSTM commented Apr 5, 2024

I agree with you that it would not make sense to document in this plugin all the data that any variant of GDB debugging might use. It should primarily document the schema expected by the CDT-GDB Adapter.
However, if an option is also required by other extensions in the debug configuration and is not documented, then this option will not be suggested via "Ctrl+Space" in launch.json and will be underlined with a yellow squiggly line. So the user must know the exact name of the option and the expected type of the value.
My vision is that to promote and facilitate the use of the debug extensions provided by Eclipse Cloud, it could be interesting to document only those as well (perhaps on a case-by-case basis, if necessary). Of course, they should not be added to the snippets but only documented.

@FlorentVSTM
Copy link
Author

Hi @colin-grant-work,
I think I found a better way to add this attribute. We can extend the JSON schema defined in cdt-gdb-vscode directly from the vscode-peripheral-inspector extension so that the "definitionPath" attribute is available in the schema only if the vscode-peripheral-inspector extension is installed.
If you think this leads to cleaner separation, I can propose a pull request in the peripheral-inspector repo and check with Rob.

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Apr 24, 2024

We can extend the JSON schema defined in cdt-gdb-vscode directly from the vscode-peripheral-inspector extension so that the "definitionPath" attribute is available in the schema only if the vscode-peripheral-inspector extension is installed.

Perfect - this is exactly what I was hoping for: I think extending it when the extension is known to be used is exactly the right approach. I apologize that I didn't get around to finding it myself, but I'm glad to know it's possible.

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.

3 participants