-
Notifications
You must be signed in to change notification settings - Fork 204
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
support modifying default keybindings prototype #356
Changes from 1 commit
8e02b15
2641b13
2375451
600c226
ff37975
01755fc
994578d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package keys | |
import ( | ||
"github.com/charmbracelet/bubbles/help" | ||
"github.com/charmbracelet/bubbles/key" | ||
log "github.com/charmbracelet/log" | ||
|
||
"github.com/dlvhdr/gh-dash/config" | ||
) | ||
|
@@ -84,7 +85,7 @@ func (k KeyMap) QuitAndHelpKeys() []key.Binding { | |
return []key.Binding{k.Help, k.Quit} | ||
} | ||
|
||
var Keys = KeyMap{ | ||
var Keys = &KeyMap{ | ||
Up: key.NewBinding( | ||
key.WithKeys("up", "k"), | ||
key.WithHelp("↑/k", "move up"), | ||
|
@@ -158,3 +159,52 @@ var Keys = KeyMap{ | |
key.WithHelp("q", "quit"), | ||
), | ||
} | ||
|
||
// Rebind will update our saved keybindings from configuration values. | ||
func Rebind(universal, issueKeys, prKeys []config.Keybinding) error { | ||
dlvhdr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
err := rebindUniversal(universal) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = rebindPRKeys(prKeys) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func rebindUniversal(universal []config.Keybinding) error { | ||
for _, kb := range universal { | ||
log.Debug("Rebinding key", "builtin", kb.Builtin, "key", kb.Key) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be adding to prs and issue keys as well. |
||
switch kb.Builtin { | ||
case "up": | ||
dlvhdr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Keys.Up = kb.NewBinding(&Keys.Up) | ||
case "down": | ||
Keys.Down = kb.NewBinding(&Keys.Down) | ||
case "firstline": // should this be first-line or firstLine? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using camel case for keys in next commit. |
||
case "lastline": | ||
case "togglepreview": | ||
case "opengithub": | ||
case "refresh": | ||
case "refreshall": | ||
case "pagedown": | ||
case "pageup": | ||
case "nextsection": | ||
case "prevsection": | ||
case "switchview": | ||
Keys.SwitchView = kb.NewBinding(&Keys.SwitchView) | ||
case "search": | ||
case "copyurl": | ||
case "copynumber": | ||
case "help": | ||
case "quit": | ||
default: | ||
// TODO: return an error here | ||
return nil | ||
} | ||
} | ||
|
||
return nil | ||
} |
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.
Do you expect people to want alts? Is this a personal need of yours? I don't expect many users will actually define multiple keys for the same action.
If there's not a big use case, maybe it's better to omit to reduce the config bloat
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.
Yea I wasn't entirely sure, I can't see myself particularly using it. So that is fair.
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.
Removed in latest commit