Skip to content

Conversation

@ShahzaibIbrahim
Copy link

Change title from "Skip All Breakpoints" to "Toggle Skip All Breakpoints" when toggled

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Thanks for the proposal! It makes sense to have all toggle options uniformly prefixed with "toggle" to make the effect easier to understand.

Here are some remarks with respect to this change:

  1. There are some futher NLSs for the same action that may need to be adapted (see detailed comments).
  2. With this change, the names of the actions within the Breakpoints view become inconsistent. There are other toggleable options, such as "Show Breakpoints Supported by Selected Target", which also miss a "Toggle" prefix.
    image
  3. To me "Toggle Skip All Breakpoints" sounds a bit odd, because in contrast to all other "Toggle X" actions, the "X" is not a noun here (like in "Toggle Breakpoint") but a verb-based action description. Unfortunately, I do not have a really good alternative either. "Toggle Skipping All Breakpoints" is probably not much better...
  4. There seems to be explicit documentation for this action, which probably has to be updated when changing the action name, e.g.: https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/4b5fccff1c7878e02430c22ede26c8788c60896e/eclipse.platform.common/bundles/org.eclipse.jdt.doc.user/reference/views/breakpoints/ref-skipall_viewaction.htm#L8

@HeikoKlare HeikoKlare changed the title Eclipse-JDT #15: Change title from "Skip All Breakpoints" to "Toggle Skip All Breakpoints" when toggled Change title from "Skip All Breakpoints" to "Toggle Skip All Breakpoints" when toggled Nov 20, 2023
@HeikoKlare HeikoKlare changed the title Change title from "Skip All Breakpoints" to "Toggle Skip All Breakpoints" when toggled Change action name from "Skip All Breakpoints" to "Toggle Skip All Breakpoints" Nov 20, 2023
@ShahzaibIbrahim
Copy link
Author

Thanks for the proposal! It makes sense to have all toggle options uniformly prefixed with "toggle" to make the effect easier to understand.

Here are some remarks with respect to this change:

  1. There are some futher NLSs for the same action that may need to be adapted (see detailed comments).
  2. With this change, the names of the actions within the Breakpoints view become inconsistent. There are other toggleable options, such as "Show Breakpoints Supported by Selected Target", which also miss a "Toggle" prefix.
    image
  3. To me "Toggle Skip All Breakpoints" sounds a bit odd, because in contrast to all other "Toggle X" actions, the "X" is not a noun here (like in "Toggle Breakpoint") but a verb-based action description. Unfortunately, I do not have a really good alternative either. "Toggle Skipping All Breakpoints" is probably not much better...
  4. There seems to be explicit documentation for this action, which probably has to be updated when changing the action name, e.g.: https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/4b5fccff1c7878e02430c22ede26c8788c60896e/eclipse.platform.common/bundles/org.eclipse.jdt.doc.user/reference/views/breakpoints/ref-skipall_viewaction.htm#L8

Understood. If you prefer to keep a consistent pattern with "Toggle X," you might consider using a noun that represents the action of skipping breakpoints. Here is a suggestion:

"Toggle Breakpoint Skipping"

@ShahzaibIbrahim
Copy link
Author

@vogella
Copy link
Contributor

vogella commented Dec 5, 2023

@HeikoKlare anything left here to do for @ShahzaibIbrahim?

@ShahzaibIbrahim
Copy link
Author

@HeikoKlare anything left here to do for @ShahzaibIbrahim?

@vogella Yes, I have found the existing ticket for this particular issue (See: eclipse-platform/eclipse.platform.swt#501) and the toggle "state/highlight" issue is for windows 11 only. I have one more suggestion in mind which I will propose soon, till then we can draft this PR.

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