-
Notifications
You must be signed in to change notification settings - Fork 16
Support for forge #54
Comments
Yes! This would be awesome! |
As a quick reference, I went over all the current bindings in magit-status-mode, and found the following keys still up for grabs:
None of these options seem particularly mnemonic, but for what it's worth I went with the backtick |
I haven't tried forge yet. I'm open to suggestions for the key binding. |
How about using the default |
Definitely an option but that seems more disruptive |
I think it’s nice to keep as many default bindings as possible. I do understand if you are hesitant to changing the binding for submodules though. |
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * layers/+source-control/github/packages.el (github-packages): Add forge. * layers/+source-control/github/packages.el (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update.
The issue is submodule and subtree popup bindings are a lower/uppercase pair (
But it is rather more inconvenient to type, and I imagine most users would be using forge commands more frequently than subtree/submodule ones? |
I’m fine with |
How about |
I’d prefer one key to be consistent with the other commands. |
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update. * layers/+source-control/github/packages.el (github-packages): Add forge. (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. (github/init-magithub): Disabling injecting issues and pull-requests sections if forge is installed.
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update. * layers/+source-control/github/packages.el (github-packages): Add forge. (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. (github/init-magithub): Disabling injecting issues and pull-requests sections if forge is installed.
Same here. As a side note, it would also clash with the spacemacs convention of "gh" navigating to a parent heading. |
Hm, perhaps |
I like the following although this is more drastic
I'm not sure if putting push so close to pull is a good idea, but it's easier to remember. Of course, it's going to throw people off for a bit |
I like it 👍 |
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update. * layers/+source-control/github/packages.el (github-packages): Add forge. (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. (github/init-magithub): Disabling injecting issues and pull-requests sections if forge is installed.
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update. * layers/+source-control/github/packages.el (github-packages): Add forge. (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. (github/init-magithub): Disabling injecting issues and pull-requests sections if forge is installed.
See 49978d0. Note that the changes only take affect after forge is loaded. The default bindings will be in place before that. |
Great! Thanks @justbur |
@justbur Small nitpick... Would it be possible to move the All the lowercase and uppercase letters are next to each other but |
Yes, try 49978d0 |
@justbur I think that is what I am running? Otherwise I wouldn't have forge on |
@justbur Cool, looks perfect! 👍 |
Any thoughts from people in this thread on the new bindings? It was inevitable, but the people who don't like them showed up in #60. I can address some of those problems, but I'm wondering if anyone wants me to revert them entirely here. |
I thought it would be a good idea at first, but soon realised that it was hard to keep the p/P distinction straight without stopping to think - once I accidentally typed "p -f u" by muscle memory and overwrote my local changes with the remote instead of force pushing. After that incident I decided to revert to the original bindings with |
Having a common binding suddenly change meaning upon installation/loading of a package also seems like a bad idea in terms of user experience. At the least it should be consistent -
|
ok, I'll revert it for now. |
For me, the bindings have worked out well. No wrong pushing :) I’m, of course, fine with moving to a better key binding. Perhaps |
Thanks @Linuus. I think they're sensible, but do take some getting used to. I think there are legitimate uses for |
What do you think of C (upper-case letter c)? It does not conflict with any key binding from Magit, its Evil binding ( |
As I mentioned above I don’t think |
Maybe we could use the binding that was used for The issue with |
|
I'm going with @Linuus I'm not sure macros are more useful than the other options, but you can certainly move the binding if it doesn't work for you. @syl20bnr I think @Miciah |
Fine with me. |
@justbur No worries :) I’ll try it out. And I probably have never actually used Good job 👏🏼 |
Note that Forge binds the ' key to the Forge dispatch popup, but evil-magit binds the ' key to the submodule dispatch popup (see emacs-evil/evil-magit#54), and evil-magit's key binding takes precedence with the vim editing style. Because Forge's key binding does not always work, it is omitted from the README. Note also that Magit binds the % key to its worktree dispatch popup, but Spacemacs binds the % key to the magit-gitflow popup, and Spacemacs's key binding takes precedence. Because there is no available key binding for the worktree dispatch popup, Forge's key binding under that dispatch popup is omitted from the README. * CHANGELOG.develop: * layers/+source-control/github/README.org: Update. * layers/+source-control/github/packages.el (github-packages): Add forge. (github/init-forge): Load forge after magit. Configure forge to use spacemacs-cache-directory. (github/init-magithub): Disabling injecting issues and pull-requests sections if forge is installed.
Now that forge's been released, it might be a good idea to consider what binding would be appropriate. I'm not sure if evil-magit is the best place for such a binding to be defined though, evil-collection also seems like an option.
Looking at here,
'
is the default binding, which is now used for Submodules by evil-magit.The text was updated successfully, but these errors were encountered: