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 keyboard shortcuts and menu items to run tests in debug mode #204

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

dstango
Copy link
Contributor

@dstango dstango commented Dec 14, 2023

Two new actions to resolve #43

  • Ctrl+Alt-D Runs all tests of the current class in debug mode
  • Ctrl+Alt+Shift-D Runs test of selected member in debug mode

This PR resolves the issue by adding two new actions/shortcuts instead of a configurable option in the preferences dialog (see discussion in issue). Reasoning: this way offers more flexibility to various user needs, as it allows easy on the fly switching between "run" and "debug" as needed. People who only ever want to use the new debug variant can do so by configuring the keyboard assignments to their likings.

I'm unsure if these default keys are good choices, or might conflict with other well known plugins that I'm unaware off. (Or prevent easy remembering ;-) ...)

@RoiSoleil
Copy link
Contributor

Can you review your PR to only contain the desired commit ?

@dstango
Copy link
Contributor Author

dstango commented Dec 15, 2023

Can you review your PR to only contain the desired commit ?

Sorry, will do. Did an update to master on my fork after submitting the PR and wasn't aware how it would affect the PR...

Ctrl+Alt-D Runs all tests in the current class in debug mode
Ctrl+Alt+Shift-D Runs test of selected member in debug mode

MoreUnit#43
@dstango
Copy link
Contributor Author

dstango commented Dec 15, 2023

Can you review your PR to only contain the desired commit ?

done

@RoiSoleil RoiSoleil merged commit 0d0359b into MoreUnit:master Dec 18, 2023
1 check failed
@dstango dstango deleted the issue/43 branch December 18, 2023 14:53
@Bananeweizen
Copy link
Contributor

@dstango please be aware this is not cross platform compatible. You could fix that by replacing the ctrl, alt, shift modifier keys by the meta keys that eclipse defines for this (or by having even different key shortcuts for MacOS, and a platform filter on the keybinding). See https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fextension-points%2Forg_eclipse_ui_bindings.html, which says

The recognized modifiers keys are M1, M2, M3, M4, ALT, COMMAND, CTRL, and SHIFT. The "M" modifier keys are a platform-independent way of representing keys, and these are generally preferred. M1 is the COMMAND key on MacOS X, and the CTRL key on most other platforms. M2 is the SHIFT key. M3 is the Option key on MacOS X, and the ALT key on most other platforms. M4 is the CTRL key on MacOS X, and is undefined on other platforms. Since M2+M3+ (Alt+Shift+) is reserved on MacOS X for writing special characters, such bindings are commonly undefined for platform="carbon" and redefined as M1+M3+.

@dstango
Copy link
Contributor Author

dstango commented Dec 22, 2023

@Bananeweizen thanks for making me aware of that. Unfortunately I have no MacOS experience to make a competent judgement what a proper fix would look like. Besides that I was just following the pattern of the the other keystrokes already defined ... (with the exception of "show view", which is defined as SHIFT+ALT+Q M [Windows], respectively Command+Option+Q M [Mac]).

So it looks to me, all shortcuts might be in conflict with expectations of Mac users, and would need fixing then, not only my new key bindings. Unfortunately I have no experience with MacOS, and no idea what proper key bindings would be, that would satisfy a Mac-user's expectation.

Thus I propose you might open a new issue addressing this general topic, which includes the concrete changes to the keys you propose for each command. I'd then be happy to provide a new PR with the changes you propose.

@Bananeweizen
Copy link
Contributor

Sorry for the late response. I also can't say what would be a good or typical fit for Mac users, and that's not what I'm trying to fix. I'm rather saying that by just replacing the key combination Ctrl-Alt-D as M1-M3-D (and similar) it would become usable on Mac already (and remain the same as before for the other operating systems). I don't say you should come up with secondary key combinations for Mac.
The show view command in MoreUnit does have a secondary key combination for Mac because all show view commands in the platform already have that and I wanted to be consistent when I added it.

@dstango
Copy link
Contributor Author

dstango commented Dec 28, 2023

@Bananeweizen Thanks for your response. I checked and read the eclipse link you provided again.

I see the following aspects to consider:

  • If I see it correctly, Mac's Option is equivalent to the Alt key on other platforms, and the eclipse documentation doesn't specify explicitly what happens on MacOS if we specify a binding with Alt. Do you know any detail about that?
  • If we change the definitions with Ctrl to M1 we would change the existing key bindings for Mac users: they would need to use the Command button instead of the Ctrl button (though the Mac does have a Ctrl button). Do we want that? Maybe it's even desirable, because Command is closer to the user's expectations.
  • If we go for the M1...M3 key binding definitions, we would also have to adjust the documentation accordingly, to reflect the different situation on MacOS.

Of course we could "just do it" -- yet I'd be more happy to have some input from a Mac user.
Though I fear despite the recent burst of activity on MoreUnit, we will not easily have someone with Mac experience join in by chance ...

@Bananeweizen
Copy link
Contributor

Okay, I had not noticed that detail for the Control key. Since I don't have a Mac, it's rather theoretical knowledge on my side, too. I'm okay with only doing something, if we get feedback from a Mac user.

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.

Add an option to let Ctrl + R run tests in debug mode
3 participants