[Debug] Add checkbox to toggle display of shortcut numbers in breakpoint grouping types

[SougandhS]

This PR introduces a new checkbox to toggle the display of shortcut numbers in the selection list for breakpoint grouping types. By default, shortcut numbers will be shown but it can be disabled for ensuring consistency with other selection lists that do not display these numbers. Users can enable the shortcut numbers by checking the new checkbox, providing more flexibility while maintaining UI consistency.

Before :

After :
By default it will show the shortcut numbers



Added a checkbox to enable or disable



Unchecking the option will result in list with no shortcut numbers ensuring consistency with other selection lists that do not display these numbers

[github-actions]

Test Results

 1 599 files  +  385   1 599 suites  +385   1h 34m 42s ⏱️ + 29m 11s
 4 173 tests ±    0   4 150 ✅ +   23   23 💤  - 23  0 ❌ ±0 
12 001 runs  +2 460  11 835 ✅ +2 415  166 💤 +45  0 ❌ ±0 

Results for commit 75ec99f. ± Comparison against base commit e4ebf10.

♻️ This comment has been updated with latest results.

[SougandhS]

Hi @merks , could you please check this small PR when you have time ?

[merks]

Those numbers act as accelerators though and I think those names don't come from menu item text where an & could specify the accelerator. So I'm not sure removing them is a good idea. Even File -> Recent Files has such numbers for a similar reason...

[SougandhS]

Those numbers act as accelerators though and I think those names don't come from menu item text where an & could specify the accelerator. So I'm not sure removing them is a good idea. Even File -> Recent Files has such numbers for a similar reason...

Thanks for the feedback! Just to clarify, this change does affect the functionality in that it allows users to disable the shortcut numbers (and accelerators) in the breakpoint grouping list. By default, the shortcut numbers will be enabled, but if users prefer to disable them, they can easily do so by unchecking the new option in the UI.

[merks]

I don't think that you've properly tested that these changes will be preserved on restart. In particular you should look carefully at what org.eclipse.debug.internal.core.Preferences.setDefaultBoolean(String, String, boolean) does versus what org.eclipse.debug.internal.core.Preferences.setBoolean(String, String, boolean, IScopeContext) does. I don't think it's generally a good idea to call org.eclipse.debug.internal.core.Preferences.getDefaultBoolean(String, String, boolean) except to determine the default value of the preference. When saving the value, it should be saved in the instance scope so when asking for the preference value, you should look it up in instance scope.

E.g.,

I question whether the extra logic here will really be of benefit to a significant number of users, but that's just my sense. In any case, the current way to looking up and storing the preference when the user does change it is not correct.

[SougandhS]

I don't think that you've properly tested that these changes will be preserved on restart. In particular you should look carefully at what org.eclipse.debug.internal.core.Preferences.setDefaultBoolean(String, String, boolean) does versus what..

Thanks for checking Restart case and code suggestion. I have made the changes accordingly and tested the restart case 👍

[merks]

I think we should get at least one other committer to review this.

@iloveeclipse

Do you have an opinion about the functionality, the names, and the labels?

[iloveeclipse]

Do you have an opinion about the functionality, the names, and the labels?

I'm mostly neutral here, I've never used / noticed the shortcuts shown and would not miss them if they would be hidden. But code wise looks OK and defaults are not changed, so LGTM.

[merks]

I'm fine once @iloveeclipse approves.

[SougandhS]

Hi @iloveeclipse , I have made the changes you suggested.

[SougandhS]

Thank you @iloveeclipse & @merks