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.

Sign up for GitHub

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

Jump to bottom

support modifying default keybindings prototype #356

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+211 −19
Draft

support modifying default keybindings prototype #356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e02b15
support modifying default keybindings prototype
miniscruff Apr 14, 2024
2641b13
Further work on custom keybindings
miniscruff May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 30 additions & 6 deletions config/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"reflect"
"strings"

"github.com/charmbracelet/bubbles/key"
"github.com/charmbracelet/lipgloss"
"github.com/go-playground/validator/v10"
"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -102,13 +103,35 @@ type Defaults struct {
}

type Keybinding struct {
Key string `yaml:"key"`
Command string `yaml:"command"`
Key string `yaml:"key"`
Keys []string `yaml:"keys"`
Command string `yaml:"command"`
Builtin string `yaml:"builtin"`
}

func (kb Keybinding) NewBinding(previous *key.Binding) key.Binding {
helpDesc := ""
if previous != nil {
helpDesc = previous.Help().Desc
}

if len(kb.Keys) > 0 {
Copy link
Owner

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in latest commit

return key.NewBinding(
key.WithKeys(kb.Keys...),
key.WithHelp(strings.Join(kb.Keys, "/"), helpDesc),
)
}

return key.NewBinding(
key.WithKeys(kb.Key),
key.WithHelp(kb.Key, helpDesc),
)
}

type Keybindings struct {
Issues []Keybinding `yaml:"issues"`
Prs []Keybinding `yaml:"prs"`
Universal []Keybinding `yaml:"universal"`
Issues []Keybinding `yaml:"issues"`
Prs []Keybinding `yaml:"prs"`
}

type Pager struct {
Expand Down Expand Up @@ -257,8 +280,9 @@ func (parser ConfigParser) getDefaultConfig() Config {
},
},
Keybindings: Keybindings{
Issues: []Keybinding{},
Prs: []Keybinding{},
Universal: []Keybinding{},
Issues: []Keybinding{},
Prs: []Keybinding{},
},
RepoPaths: map[string]string{},
Theme: &ThemeConfig{
Expand Down
52 changes: 51 additions & 1 deletion ui/keys/keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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"),
Expand Down Expand 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 {
Copy link
Owner

Choose a reason for hiding this comment

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

do you mean to handle issueKeys in a later PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It matches the pr keys stuff so I just didn't handle it yet with the questions I had, but it will be part of this PR.

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)
Copy link
Owner

Choose a reason for hiding this comment

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

nice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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":
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can use the keys.Keys global var here instead of the switch case.
If you don't like updating a global var, you can expose a method in the keys package. I think this will be automatically picked up in the ui.go file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The switch is used to map the yaml string to the keybinding in the global var. I am not sure how to go bind the two otherwise. Other then maybe reflection.

Keys.Up = kb.NewBinding(&Keys.Up)
case "down":
Keys.Down = kb.NewBinding(&Keys.Down)
case "firstline": // should this be first-line or firstLine?
Copy link
Owner

Choose a reason for hiding this comment

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

I think firstLine would be more readable

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
}
43 changes: 43 additions & 0 deletions ui/keys/prKeys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keys

import (
"github.com/charmbracelet/bubbles/key"
"github.com/dlvhdr/gh-dash/config"
)

type PRKeyMap struct {
Expand Down Expand Up @@ -74,3 +75,45 @@ func PRFullHelp() []key.Binding {
PRKeys.WatchChecks,
}
}

func rebindPRKeys(keys []config.Keybinding) error {
Copy link
Owner

Choose a reason for hiding this comment

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

I see there's the SetKeys method on the KeyBinding so maybe you can update the global Keys struct that way?

for _, prKey := range keys {
var k key.Binding

switch prKey.Builtin {
case "assign":
k = PRKeys.Assign
case "unassign":
k = PRKeys.Unassign
case "comment":
k = PRKeys.Comment
case "diff":
k = PRKeys.Diff
case "checkout":
k = PRKeys.Checkout
case "close":
k = PRKeys.Close
case "ready":
k = PRKeys.Ready
case "reopen":
k = PRKeys.Reopen
case "merge":
k = PRKeys.Merge
case "watchchecks":
k = PRKeys.WatchChecks
default:
// TODO: return an error here
return nil
}

// Not really sure if this is the best idea but I am not
// sure how else we are meant to define alt keys.
if len(prKey.Keys) > 0 {
k.SetKeys(prKey.Keys...)
} else {
k.SetKeys(prKey.Key)
}
}

return nil
}
23 changes: 17 additions & 6 deletions ui/ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
)

type Model struct {
keys keys.KeyMap
keys *keys.KeyMap
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am actually not sure this is used, it should be a duplicate of the pre-created KeyMap from the keys.go file.

sidebar sidebar.Model
prSidebar prsidebar.Model
issueSidebar issuesidebar.Model
Expand Down Expand Up @@ -82,10 +82,7 @@ func NewModel(configPath string) Model {
}

func (m *Model) initScreen() tea.Msg {
var err error

config, err := config.ParseConfig(m.ctx.ConfigPath)
if err != nil {
showError := func(err error) {
styles := log.DefaultStyles()
styles.Key = lipgloss.NewStyle().
Foreground(lipgloss.Color("1")).
Expand All @@ -107,10 +104,24 @@ func (m *Model) initScreen() tea.Msg {
"err",
err,
)
}

cfg, err := config.ParseConfig(m.ctx.ConfigPath)
if err != nil {
showError(err)
return initMsg{Config: cfg}
}

err = keys.Rebind(
cfg.Keybindings.Universal,
cfg.Keybindings.Issues,
cfg.Keybindings.Prs,
)
if err != nil {
showError(err)
}

return initMsg{Config: config}
return initMsg{Config: cfg}
}

func (m Model) Init() tea.Cmd {
Expand Down