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

Change Commit shortcut to ⌘+Return #845

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

danshevluk
Copy link
Contributor

Hey!
I've been using GitUp for a long time for all kinds of tasks and thought of a small improvement that could improve most users' workflow.

This PR changes the shortcut for creating a commit from the AdvancedCommitView to Cmd+Return instead of Opt+Return. Usage of a Cmd+Return is more common in modern macOS apps, so it makes sense to update option usage to a more familiar key binding.

Similar shortcuts could be found in other Git GUI like Fork, Tower, or GitKraken.
Keycombiner provides a convenient way to compare all three:

To complete this task it's needed to update wiki page with sourtcuts, but it's impossible to do in a PR, so please help me with that.

Looking forward to your feedback!

@lucasderraugh
Copy link
Collaborator

Thanks for the contribution! While I agree that it is the more natural shortcut, I'm also a bit concerned with dropping the old way considering the whole app uses option as the main modifier (for better or worse). Perhaps there is a way to preserve option enter in addition to adding command. I'll have to think about it a bit. Anyone else is willing to comment how they feel as well.

@peteschaffner
Copy link

I've been maintaining a personal fork of GitUp and have made this same change (in fact I didn't realize the option modifier binding was in place as it isn't a shortcut I've encountered before in situations with a textfield + primary button).

As @lucasderraugh mentioned, it is the main modifier throughout, so if the change were to be made, it should really be done everywhere. (Look at all the view controller xibs that are presented as modal views via runModalView:withInitialFirstResponder:completionHandler:.)

I also agree that if opt-return could be maintained, it would be great for those who have become accustomed to it (although I'm not sure how to go about that).

@danshevluk
Copy link
Contributor Author

Thanks for the feedback! I'll try to find a way to support both shortcuts and will check for other similar actions 👍

@danshevluk
Copy link
Contributor Author

danshevluk commented Jun 28, 2022

OK, so after some research, it appears that there are a lot of places with the same behavior. Currently those are:

-> GIAdvancedCommitViewController.xib

  • Commit

-> GICommitRewriterViewController.xib

  • Rewrite Commit

-> GICommitSplitterViewController.xib

  • Continue…
  • Split Commit

-> GISimpleCommitViewController.xib

  • Commit All Changes

I tried to find solutions to support both keyEquivalents, but could not find any easy options. There is a way to add an additional menuItem with the same actions but different key binding. This could work for some actions, but not as a whole-project solution.

If you agree that it's an important addition to GitUp, then there are two ways that we achieveachive that:

  • Just change the shortcut in all mentioned places to cmd+return. It's easy to do, but could break old shortcuts for lots of users ⚠️
  • Move keyEquivalents setup to .m files from .xib and switch between option/cmd based no user preferences. For all new users it could be set to cmd, and to keep old users happy they will be able to opt-in in preferences, but not automatically.

Between these two options, I irrationally tend more to make a breaking change. The original way is strange and hard to discover without specifically looking into documentation.

How do you feel about these options?

@peteschaffner
Copy link

I would also just make the breaking change in this particular situation.

(I suppose one way of teaching about the change would be to throw an alert when the old command is used, explaining the new modifier.)

@lucasderraugh
Copy link
Collaborator

lucasderraugh commented Jul 1, 2022

Alright, let's go with the breaking change of using command for the commit editors everywhere in the app. I'll play around with it before pushing an update to see if there needs to be any type of coaching.

If you want to replace the shortcut everywhere it's applicable please do and then I'll merge this in before next release.

Usage of a cmd+return is more common in modern macOS apps, so it makes sense to update option usage to a more familiar key binding.
@danshevluk danshevluk force-pushed the change-commit-shortcut branch from b425b7b to ec3a0ca Compare July 2, 2022 13:25
@danshevluk
Copy link
Contributor Author

@lucasderraugh done! I updated all mentioned shortcuts to cmd+return as discussed. Please let me know if you have any further suggestions ✅

@braver
Copy link

braver commented Jul 2, 2022

I'd just like to chime in to say this is pretty massive. I use several tools to work with Git repos and my fingers get tangled up because Gitup is different from everything else, especially for submitting commit messages.

🎉 👍🏻

@lucasderraugh
Copy link
Collaborator

Thanks @danshevluk. I want to test out some other libgit changes that I've needed to get in for some time before we put this in, but I'll pick this up after that.

@lucasderraugh
Copy link
Collaborator

Sorry for the long delay on this. Just pushed 1.3.3. to the continuous channel. I would like to work out some kind of alert dialog that shows once for users informing them of this change as it is a muscle memory changer for sure.

I'll try to think of some simple UI and how it can be tracked, but I'm just busy with lots of other things unfortunately so it might not be right away.

<rect key="frame" x="135" y="8" width="847" height="16"/>
<autoresizingMask key="autoresizingMask" widthSizable="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" controlSize="small" lineBreakMode="truncatingTail" allowsUndo="NO" sendsActionOnEndEditing="YES" alignment="left" title="&lt;TITLE&gt;" id="rrE-b8-3j3">
<font key="font" metaFont="controlContent"/>
<font key="font" metaFont="cellTitle"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is it right change? in other places there is controlContent font.

@richardtop
Copy link

Very interested in this change. Would feel more intuitive to use compared to the default.

@prostolyubo
Copy link
Contributor

Same, cmd is much better modifier.

@lucasderraugh lucasderraugh merged commit 519ef0e into git-up:master Feb 9, 2024
@danlucraft
Copy link

Thank you for this! For years I've been hitting Cmd+Return in gitup to commit and every time hearing the bonk noise 😄

@richardtop
Copy link

Now I find it difficult to switch back...

@ilyasotkov
Copy link

Surely this change will feel more intuitive for new users, but for people who've been using GitUp daily for years, this feels like banging your head against a wall every time you're trying to make a commit because there's some serious muscle memory already built up.

Would be great to be able to have to option to go back to Opt+Return.

@tockrock
Copy link

Thanks for this! I appreciate the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

10 participants