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

[#943] Provide API for "org.eclipse.cdt.debug.gdbjtag.core.jtagDevice" #990

Closed
wants to merge 1 commit into from

Conversation

ruspl-afed
Copy link
Member

  • add IGDBJtagConstants.ATTR_JTAG_DEVICE and remove duplicated definitions

Fixes #943

….jtagDevice"

* add `IGDBJtagConstants.ATTR_JTAG_DEVICE` and remove duplicated
definitions
Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruspl-afed
Copy link
Member Author

@jonahgraham after reading both https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CHANGELOG-API.md and https://bugs.eclipse.org/bugs/show_bug.cgi?id=566462 I still miss the rationale why this was done.
Perhaps, because we want to switch to jtagDeviceId and stop spreading the jtagDevice. But in this case we need to first stop spreading the jtagDevice through our own codebase. Perhaps we should provide a common way to resolve GDBJtagDeviceContribution instead of duplicating the same code for core and UI

@jld01 could you please provide your opinion here?

@jonahgraham
Copy link
Member

Perhaps we should provide a common way to resolve GDBJtagDeviceContribution instead of duplicating the same code for core and UI

This exists already in GDBJtagDeviceContributionFactory.getInstance().findByDeviceId - the duplication around the call sites of findByDeviceId are because the attributes are stored in a different location in UI and core parts (ILaunchConfiguration.getAttribute vs a map called attributes - which is populated by a combination of the launch configuration and other attributes). The other duplication is handling of the deprecated attribute, but that code could eventually be deleted and is only needed for compatibility with old launch configurations.

@jld01
Copy link
Contributor

jld01 commented Jan 6, 2025

@ruspl-afed the jtagDeviceId launch attribute was introduced with https://bugs.eclipse.org/bugs/show_bug.cgi?id=535024 to allow localization of JTAG device names presented in the UI while maintaining the portability of launch configurations. The old jtagDevice launch attribute was deprecated and eventually removed from API but is still defined privately by both GDBJtagDSFFinalLaunchSequence and GDBJtagDSFDebuggerTab to support the continued use and possible upgrading of legacy launch configurations that do not currently include a JTAG device ID.

I agree that a common definition of the legacy jtagDevice attribute would be more elegant in some ways, but the value of these strings will never change.

Reintroduction of this attribute as API would not be appropriate.

@ruspl-afed ruspl-afed closed this Jan 6, 2025
@ruspl-afed ruspl-afed deleted the 943 branch January 7, 2025 10:05
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.

Provide API to reference "org.eclipse.cdt.debug.gdbjtag.core.jtagDevice" launch attribute
3 participants