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

Configurable keybindings #48

Open
vidosits opened this issue Jul 1, 2019 · 10 comments
Open

Configurable keybindings #48

vidosits opened this issue Jul 1, 2019 · 10 comments
Labels
enhancement New feature or request hacktoberfest

Comments

@vidosits
Copy link

vidosits commented Jul 1, 2019

I love lazydocker. Good job, Jesse!

Is your feature request related to a problem? Please describe.
Default keybindings are hard coded. Problem is, not every keyboard has [ and ] keys.

Describe the solution you'd like
Make the keybindings configurable.

Describe alternatives you've considered
Move the previous/next tab keybindings from [ and ] to something else.

@vidosits vidosits added the enhancement New feature or request label Jul 1, 2019
@glvr182
Copy link

glvr182 commented Jul 2, 2019

A way to solve this is to use the config file and this little library

@jesseduffield
Copy link
Owner

jesseduffield commented Jul 5, 2019

very cool library! I wonder how best to go about this: we currently have most (though not all) of our keybindings defined in one place, so for those which are stored there, it would not be hard to merge in what we have defined in the user config. In fact even in the other places it wouldn't be hard just to refer to the user config.

in config.yml we can have:

keybindings:
  stopContainer: 's'
  nextTab: '['
  previousTab: ']'

then in keybindings.go we can have:

		{
			ViewName:    "containers",
			Key:         keybinding.MustParse(gui.Config.UserConfig.Keybindings.StopContainer),
			Modifier:    gocui.ModNone,
			Handler:     gui.handleContainerStop,
			Description: gui.Tr.Stop,
		},

How does that approach sound? Only real downside with that approach is that you don't get to specify if you want to use the gocui.ModAlt modifier, but we aren't currently using that anywhere anyway.

Another open question would be: how general should the keybindings be? Should there be one key for 'stop' or should we have 'stopContainer' and 'stopService'?

@glvr182
Copy link

glvr182 commented Jul 5, 2019

That approach sounds good, we could indeed look into the modAlt, but that is an issue for the library i guess. I could spend this weekend on it?

As for how general they should be, I think at first the best thing is to include everything to avoid bug creation. If we find that they are "too" general, they can always be stripped away.

@jesseduffield
Copy link
Owner

I agree @glvr182 , and if you want to spend time on this that would be awesome :)

@jesseduffield
Copy link
Owner

Also worth noting that if we set a key to be the empty string, that will enable the user to disable certain keys, which satisfies #27

@jesseduffield
Copy link
Owner

jesseduffield commented Jul 5, 2019

also worth noting we'd need to support arrays of keys which luckily is already covered by that library. And if we're supporting arrays of keys then I guess we'd need to have it look like this:

keybindings:
  stopContainer: ['s']
  nextTab: ['[']
  previousTab: [']']

So that we can continue to use the simple config merging logic

@glvr182
Copy link

glvr182 commented Jul 5, 2019

@jesseduffield Do you think we should use a map or a struct for the config?

@jesseduffield
Copy link
Owner

I think a struct works better, that way we have strong typing. I made the decision to switch the i18n stuff to a struct and it's already proven really helpful in telling me when things are out of sync :)

@ser-drephs
Copy link

Any news on this one?

@jesseduffield
Copy link
Owner

This is totally do-able: we can just copy the approach that lazygit uses: https://github.com/jesseduffield/lazygit/blob/master/pkg/config/user_config.go

Any takers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants