-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Custom commands submenus #4324
Custom commands submenus #4324
Conversation
docs/Custom_Command_Keybindings.md
Outdated
customCommands: | ||
- key: X | ||
description: "Copy/paste commits across repos" | ||
subCommands: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure subCommands
is the best name for the key. Better suggestions welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about commandMenu
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, see comments
docs/Custom_Command_Keybindings.md
Outdated
description: "Paste selected commits from clipboard" | ||
``` | ||
|
||
If you use the subCommands property, none of the other properties except key and description must be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use the subCommands property, none of the other properties except key and description must be used. | |
If you use the subCommands property, none of the other properties except key and description can be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️ da74a08
docs/Custom_Command_Keybindings.md
Outdated
customCommands: | ||
- key: X | ||
description: "Copy/paste commits across repos" | ||
subCommands: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about commandMenu
?
return self.showCustomCommandsMenu(customCommand) | ||
} | ||
bindings = append(bindings, &types.Binding{ | ||
ViewName: "", // custom commands menus are global; we filter the commands inside by context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not allow optionally specifying a context on a top-level custom command menu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question; complexity? This would give us two levels of filtering by context, one for the menu itself and then again for the commands inside. I think this could be confusing, I find it easier to understand if menus are always global.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that it would be confusing, but I also don't mind not supporting it at the moment. We can wait and see if the use case comes up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking about cases like this:
customCommands:
- context: files
commandMenu:
- command: whatever
context: commits
This sub-command would just never show up in the menu, ever. And I think you could easily run into cases like that in practice when you start to restructure your menus and move commands from one to another.
We could probably add validation to catch and disallow such cases, but it's actually not trivial if you think about the details, and I don't think it's worth it.
This was backwards; we renamed Sha to Hash, so Sha is now deprecated, not Hash.
It has been 'e' instead of 'o' for quite a while now.
It is false by default. This way there's one less change to make in the next commit.
This allows us to tell whether they appear in the user's config file, which we will need later in this branch.
We'll reuse it in the next commit.
b8f0b9e
to
82658ff
Compare
Addressed the feedback, mentioned the individual fixups above; will squash now for easier re-review. |
82658ff
to
df17896
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.47.2` -> `v0.48.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.48.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.48.0) [Compare Source](jesseduffield/lazygit@v0.47.2...v0.48.0) <!-- Release notes generated using configuration in .github/release.yml at v0.48.0 --> #### What's Changed ##### Enhancements 🔥 - Custom commands submenus by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4324 ##### Maintenance ⚙️ - Refactor migrations to only marshall yaml twice by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4318 **Full Changelog**: jesseduffield/lazygit@v0.47.2...v0.48.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODIuNCIsInVwZGF0ZWRJblZlciI6IjM5LjE4Mi40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
I want to be able to configure custom commands that I don't need very often; I don't want these to pollute the global keybindings menu, and I don't want to assign globally unique keybindings to them (because there are only so many of these available, and also because I wouldn't be able to remember them, because the commands are not used often). However, I still want to invoke them through keybindings somehow.
I find that the perfect solution for this is to configure a menu that contains custom commands. I can pop open the menu using only one key that I need to remember, but I can access the individual custom commands inside using keys that don't need to be unique with the rest of the global keybindings.
In this PR we achieve this by adding an optional
subCommands
property to customCommand that can be used instead of the other properties likecommand
, etc. This is an alternative approach to #4276, which added a new top-level property for custom command menus.Potentially closes #3799.
go generate ./...
)