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

Keymaps #1509

Closed
wants to merge 32 commits into from
Closed

Keymaps #1509

wants to merge 32 commits into from

Conversation

ppot
Copy link
Contributor

@ppot ppot commented Feb 15, 2017

Keymaps Feature

On this implementation, I included a base of Keymap following three specific files.

darwin.json
win32.json
linux.json

This way, the Keymap will be easier to manage using accelerator. Since we want the Keymap to be easily modified and not forcing the remplacement of metaKey depending of the operating system.

New file structure

.hyper/config.js
.hyper/plugins
.hyper/local

New DEV structure

You can now do development when running Hyper locally and the DEV folder will not be created in the production build

  • config will be at .hyper/DEV/config.js

Resolve

#233 Tmux bind `
#657 make keyboard shortcuts configurable
#783 Rich Support for Tweaking Modifier Keys
#833 Most common readline keybinding don't work (ctrl-a, ctrl-e, ctrl-w etc.)
#872 Flexible keymap (hotkeys / shortcut)
#1069 Ctrl+X not working in nano
#1100 CTRL+a does not work in tmux under Linux
#1120 1.0.0 release broke some keybindings
#1178 Change realod accelerator
#1185 Shortcuts break when using Alt key as Meta
#1199 Ctrl + W closes the terminal
#1200 [windows] Control characters not working!
#1279 Git automerge - nano edit fail
#1361 Hyperterm doesn't handle Alt key combinations correctly
#1424 Running nano from Hyper on Windows, doesn't allow me to exit with CTRL+X.
#1584 Plugins Existences
#1589 Crashes on open with invalid config
#1627 close on CTRL-W
#1776 Gaze is adding all folders in HOME to watch list
#1782 JOE Editor and Hyper, disabling shortcuts for proper use

@ppot
Copy link
Contributor Author

ppot commented Feb 15, 2017

@rauchg @matheuss @albinekb Here is a simpler keycap if you can review it.

@ppot
Copy link
Contributor Author

ppot commented Feb 15, 2017

@iamstarkov if you prefer to look at a simpler keymap for now

.gitignore Outdated
@@ -6,6 +6,7 @@ node_modules

# logs
npm-debug.log
.DS_Store
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't belong in .gitignore as it's not a build artifact

app/index.js Outdated
@@ -47,9 +47,12 @@ const fileUriToPath = require('file-uri-to-path');
const isDev = require('electron-is-dev');

// Ours
const Keymap = resolve(__dirname, '../keymaps/darwin.json');
console.log(Keymap);
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please

@rauchg
Copy link
Member

rauchg commented Feb 16, 2017

The only thing that's not clear to me is:

  • How do users change the keymap config?
  • How do plugin authors change the keymap config?

@ppot
Copy link
Contributor Author

ppot commented Feb 17, 2017

The current implementation lay ground for implementing an edit commands depending on the keybinding.

Allowing the keymap to be change is a different issue and this one.

Copy link
Contributor

@iamstarkov iamstarkov left a comment

Choose a reason for hiding this comment

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

looks good to me

accelerator: commands['window:minimize']
},
{
role: 'zoom'
Copy link
Contributor

Choose a reason for hiding this comment

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

this is confusing. Shall we have descriptive Label here and relevant hotkey?

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 should make a key for the zoom that can override the default yes

"pane:close":"Ctrl+Shift+W",

"editor:undo":"Ctrl+Shift+Z",
"editor:redo":"Ctrl+Shift+Z",
Copy link

Choose a reason for hiding this comment

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

I guess this should change to a different key. Ctrl+Y is regularly used as redo, so Ctrl+Shift+Y? Same for the Windows 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.

Yeah.

@rauchg
Copy link
Member

rauchg commented Mar 2, 2017 via email

@chabou
Copy link
Collaborator

chabou commented Mar 7, 2017

@ppot "Allowing the keymap to be change is a different issue and this one.". You mean Plugins can't use it yet ?

@ppot
Copy link
Contributor Author

ppot commented Mar 8, 2017

@chabou Yes, I didn't made plugins be able to update the keymap. Since I have no use case of plugins needing some keybinding. If you have a plugin in head needing this. I can test and update it.

@iamstarkov
Copy link
Contributor

@ppot the cases for plugins to maintain hotkeys is hotkeys packs. For people migrating from iTerm2 there will be a package hyper-iterm2-hotkeys and similar

@chabou
Copy link
Collaborator

chabou commented Mar 8, 2017

My plugin hyper-pane need some keybindings :
It would be awesome to register some hotkeys without using mousetrap and reattach keys each times that focus has changed : https://github.com/chabou/hyper-pane/blob/master/index.js#L420-L455

I have chosen the same way than @iamstarkov to attach my keys.
Another way is to tweak hterm internal keyboard like this but I REALLY prefer our way.

@iamstarkov
Copy link
Contributor

yep, hyper-pane case is legit

@chabou
Copy link
Collaborator

chabou commented Mar 8, 2017

It would be great to register to up, down and/or press event. In my plugin case, I would like to show shortcut indicator (with pane number), only when modifiers (ie. ctr+alt by default), and only them, are down.

RegisterKeys exported function should be evaluated after every config change.

I have a good idea of what plugins need. But maybe it should take place in another PR, after this one have been merged.

@iamstarkov
Copy link
Contributor

@ppot another thing about allowing plugins to influence keymap is that, right now plugins doesnt override hyper's hotkeys. lets say, its enough for me to switch tabs with ctrl+tab and i dont care about cmd+alt+arrows for this. But i do want to navigate through panes with cmd+alt+arrows. so i can try (without luck, obviously) to remap hyper-panes hotkeys to this:

  paneNavigation: {
    navigation: {
      up:    'command+option+up',
      down:  'command+option+down',
      left:  'command+option+left',
      right: 'command+option+right'
    },
  },

but as far as hyper's electron menu's shorcuts has prio over mousetrap hotkeys, it doesnt work out.

I believe its valid use case.

@iamstarkov
Copy link
Contributor

iamstarkov commented Mar 10, 2017

demo of aforementioned case chabou/hyper-pane#6

@coccyx
Copy link

coccyx commented May 24, 2017

Wondering if this project is actively maintained? It seems impossible there are this many open issues referencing the same problem, with a user submitted PR, which goes weeks without any activity. I've kind of given up on waiting and decided I'll just live with the stock Windows terminal. What is the point in putting out binaries for all the other platforms other than MacOS if you can't even reverse-i-search in Bash?

@danielbertolozi
Copy link

Reinforcing what @coccyx just mentioned.
I use Vim both at home/university (Mac) and at work (Linux). At this point, it is impossible to use Hyper on my Linux, as lots of keybindings that use the Ctrl modifier are captured by Hyper - I can't cycle windows without it closing everything. Going back to Gnome Terminal for now.

@mqsoh
Copy link

mqsoh commented May 24, 2017

Dereinforcing what others have mentioned, I'd like to thank @ppot and anyone else that worked on this ticket! I eagerly await a release with these changes and don't mind using my dumb old terminal for now.

@timothyis
Copy link
Contributor

I just want to take a minute to say that Hyper is always being talked about by the core team in the zeit.chat slack channel. It's a tough one because we all have full-time jobs and ZEIT are super focused and busy working on the things that maintain their growth for now. They (ZEIT) actively try and find time to work on Hyper. Keymaps is a big PR and is hard to review properly, so I just want everyone to know that it is being talked about every day. ❤️

@ppot ppot mentioned this pull request May 25, 2017
@albinekb
Copy link
Contributor

albinekb commented May 26, 2017

This is now moving, part 1 is in master: #1867, part 2 coming: #1876 & more 💯

Locked due to github can't handle all this data in one view, page takes 20s to load from their servers....

@ppot
Copy link
Contributor Author

ppot commented Jun 3, 2017

Good news. Part 2 is in master with #1876!

@vercel vercel locked and limited conversation to collaborators Jun 14, 2017

<h2 id="keymaps"><a href="#keymaps">Keymaps</a></h2>

<P> All commands keys can be changed. In order to change them, edit
Copy link
Contributor

Choose a reason for hiding this comment

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

All command keys can be changed*

<h2 id="keymaps"><a href="#keymaps">Keymaps</a></h2>

<P> All commands keys can be changed. In order to change them, edit
<code>~/.hyper/config.js</code> and add to <code>keymaps</code> your desired change.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

add your desired change to <code>keymaps</code>*

<code>~/.hyper/config.js</code> and add to <code>keymaps</code> your desired change.</p>
<p> Then Hyper will change the default with your custom change.</p>

<p> Exemple: <code>'window:devtools': 'Cmd+Alt+O'</code> <p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Example*

@ppot
Copy link
Contributor Author

ppot commented Dec 17, 2017

As it was integrated. Closing the PR.

@ppot ppot closed this Dec 17, 2017
@ppot ppot deleted the keymaps branch December 17, 2017 01:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
‼️ Priority: OMG Maximum Issue needs to be seen to immediately. Like now. Please. ⚠️ 🙅‍♀️ Status: On Hold Issue or PR is on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.