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

Add modal keyboard motion mode #3315

Merged
merged 45 commits into from
Mar 18, 2020
Merged

Add modal keyboard motion mode #3315

merged 45 commits into from
Mar 18, 2020

Conversation

chrisduerr
Copy link
Member

This implements a basic mode for navigating inside of Alacritty's
history with keyboard bindings. They're bound by default to vi's motion
shortcuts but are fully customizable. Since this relies on key bindings
only single key bindings are currently supported (so no ge, or
repetition).

Other than navigating the history and moving the viewport, this mode
should enable making use of all available selection modes to copy
content to the clipboard and launch URLs below the cursor.

This also changes the rendering of the block cursor at the side of
selections, since previously it could be inverted to be completely
invisible. Since that would have caused some troubles with this keyboard
selection mode, the block cursor now is no longer inverted when it is at
the edges of a selection.

Fixes #262.

This was referenced Feb 11, 2020
CHANGELOG.md Outdated Show resolved Hide resolved
alacritty/src/input.rs Outdated Show resolved Hide resolved
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

it looks idk, "normal", like I don't really see how could you implement this thing the other way. Well, I have question about configuration though. I don't think we should provide most of those bindings to normal mode, right? maybe we should put them in a separate restricted section section?

Also, I haven't touched keyboard_motion.rs, since I don't remember whether I can review it now or no.

alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/term/mod.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/term/mod.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/term/mod.rs Show resolved Hide resolved
alacritty_terminal/src/term/mod.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/term/mod.rs Show resolved Hide resolved
alacritty/src/input.rs Outdated Show resolved Hide resolved
@kchibisov
Copy link
Member

if I type ls -lah a bunch of times, then start then enter keyboard mode, start block selection and go to top with g, selection will be messed up a bit.

@kchibisov kchibisov added this to the Version 0.4.2 milestone Feb 13, 2020
@nixpulvis
Copy link
Contributor

@kchibisov I can't seem to replicate this, after typing ls -lah 10 times. I get a block from the bottom (at the position of my cursor) to the top left of history.

@kchibisov
Copy link
Member

@nixpulvis it was stated on IRC that it’s expected vim behavior, so it’s not a problem.

@chrisduerr
Copy link
Member Author

I think I should have implemented everything now, except for a good name for the keyboard motion click action. I'll have to think about that for a bit but that shouldn't stop anyone from reviewing all the changes I have made, since it's quite a bit of stuff.

@chrisduerr
Copy link
Member Author

After some consideration, I've made the decision to remove the ability to emulate the mouse using the keyboard motion mode.

The main reason for this is that it created a lot of additional complexity, especially related to modifiers. As far as I can tell, there's no clear solution on how clicking or scrolling with modifiers should be emulated. So this would only allow a very limited amount of emulation and could easily cause confusion on what exactly is going on and how things work.

I think since we're talking about terminal emulators here, we should be able to make the assumption that the applications never rely on mouse input and always have their own solutions for keyboard controls. So we should encourage users to make use of these instead, since they allow properly doing things instead of hacking around it.

@nixpulvis
Copy link
Contributor

nixpulvis commented Feb 15, 2020

@chrisduerr I agree completely.

However, there's still something I think that's strange about the state of this PR (stemming from the branch name). I don't want to think of this as vi-mode, I want to think of this as adding modes (like vi), which use the same bindings as vi. This would be the first (two?) mode(s):

  1. Selection Mode, which is the majority of this PR.
  2. Potentially an Open URL mode, which would ideally be a bit more elegant. For example, why use word, punct, line, etc movements to get to a URL when there are only a few of them on the screen at a time? Couldn't we have a better way to do this?

I don't personally care about having multiple modes to use, as long as there are logical bindings to get to them. This would also solve a minor thing about this PR that's been bugging me, which is the inverted nature of the default mode switch bindings with respect to vi, because now I'm thinking I want to get to selection mode through a mode selection leader key which could default to Ctrl + ESC. Then I'd press s to enter selection mode.

Otherwise, we could just have two mode activation bindings, Ctrl + s and (something like) Ctrl + u. I don't want to overly complicate things too much for you, but I do think it might be worth some about of refactoring.

@chrisduerr
Copy link
Member Author

@nixpulvis I definitely think that at least a search mode is still missing, however I don't think I want to block this PR on that addition. I'd personally like to get some feedback on this before moving forward with additional features.

My idea for the progression of this feature was to implement it in an incremental nature, to get optimal feedback on how people feel about things and to make it possible to stick to minimal possible implementation.

After this, I'd like to go for a search mode for example. This search mode would likely start a selection including everything you've searched for and move your keyboard cursor to the end of the selection. I plan on starting that using the / key from within the keyboard motion mode.

Once that is implemented, it will obviously be much easier to open URLs already. You could simply run /http and it would select the first URL and you could then easily switch between them using n and N and launch it using the KeyboardMotionLaunchUrl action.

At that point, launching URLs should already be pretty simple. So I'd like to give that a run for a bit first. I'd like to find out if it makes more sense to just add a shortcut for /https, like /// maybe, or if opening URLs is still too painful and we need to add a dedicated "url mode".

Even if we do add a dedicated url mode, the implementation could still be done in multpile ways. Either by having a completely separate mode with a completely separate binding, or by adding the URL search as a 'motion' where pressing the key combination for a URL (like tridactyl/vimium) would just move the cursor to that URL.

I think it's worthwhile going slow on this because I think at this point it's difficult to say how exactly this will play out once you use it a bit. So I definitely want to give the normal search a test before implementing specialized URL search. With something like vimium there are a ton of URLs open at a time, making it an obvious necessity to have a way to jump to one specific URL immediately, but I'm not sure if the same is true for a terminal emulator.

@nixpulvis How do you feel about this approach? Do you think it would be a mistake to go for a minimal approach first and get it merged before a second iteration? If so, are you concerned about users complaining that the mode sucks and giving up on it before it ever gets good, or are you more concerned about the initial PR going in a direction that will be completely scrapped in the future?

@kchibisov
Copy link
Member

I've noticed that you're not expanding keyboard cursor on full-width glyphs properly, by that I mean that you're not doing it from spacer.

alacritty.yml Outdated Show resolved Hide resolved
alacritty.yml Outdated
# - ScrollPageUp, ScrollPageDown, ScrollHalfPageUp, ScrollHalfPageDown,
# ScrollLineUp, ScrollLineDown, ScrollToTop, ScrollToBottom
# - ToggleKeyboardMode, ToggleNormalSelection, ToggleLineSelection,
# ToggleBlockSelection, ClearSelection, KeyboardMotionLaunchUrl,
Copy link
Member

Choose a reason for hiding this comment

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

KeyboardMotionLaunchUrl if we want to support more than just opening URLs we should likely name it properly, so we don't have broken binding in a future.

Copy link
Member Author

Choose a reason for hiding this comment

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

What else do we want to support? There's nothing as far as I know.

Copy link
Member

Choose a reason for hiding this comment

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

I'm just afraid of naming things so specific, like you never know what could we support next. But we I'm not insisting on rename.

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't get me wrong, I definitely agree with you here, but what would you suggest instead?

KeyboardMotionTriggerAction? That doesn't say very well what it does though, launch url is more specific.

alacritty.yml Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/keyboard_motion.rs Outdated Show resolved Hide resolved
alacritty/src/config/bindings.rs Outdated Show resolved Hide resolved
@kchibisov
Copy link
Member

We should also update selection on actions that change cursor position, like PageUp,PageDown.

alacritty.yml Outdated Show resolved Hide resolved
@chrisduerr chrisduerr merged commit 1a8cd17 into alacritty:master Mar 18, 2020
@chrisduerr chrisduerr deleted the vimode branch March 18, 2020 02:35
@nixpulvis
Copy link
Contributor

🎉 thanks for implementing this. I can't wait to use it literally all the time!

@SRGOM
Copy link

SRGOM commented Mar 18, 2020

Speaking of launching I wonder if it would make sense to have an action to "xdg-open" the complete word where my cursor is? So I do this :

ls 
<Enter mode>  

?gi.*pdf
<Hit key to open>
?http.*duck
<Hot key to open>

@nixpulvis
Copy link
Contributor

@SRGOM I think the functionality you desire is being tracked in #2792.

@kchibisov
Copy link
Member

kchibisov commented Mar 18, 2020

No, it's not really related to URL matching directly. You can implement it in terms of selection and bindings.

ToggleSemanticSelection
LaunchThingWithSelection
ClearSelection

We don't have a way to toggle xdg-open on selected content, but I feel like it could make some sense to do that. However custom matchers are nicer, since you'll get underline, etc.

@nixpulvis
Copy link
Contributor

@kchibisov I'm assuming they want to open things that are missing the file:// bit.

I'm also a bit irritated that our default launcher doesn't seem to open relative paths, but that isn't our fault.

@SRGOM
Copy link

SRGOM commented Mar 19, 2020

@nixpulvis you're right about my wanting to open things that are missing file:// bit. The solution I'm requesting is much simpler though, without a need for a complex regex. Just \w over cursor and then an xdg-open. While xdg-open isn't properly implemented by any single program on earth and nobody knows where the defaults go, I'll still prefer a standard tool and not alacritty-specific custom openers...

@nixpulvis
Copy link
Contributor

@SRGOM The issue is that we'd like to underline the link before pressing enter to open it. This requires detecting links in some way. If I'm understanding your suggestion correctly, then I would be able to press enter on words that would not open, and we would still pass the string to xdg-open. This seems a bit unfortunate, although it would allow you to open more links perhaps.

I'm honestly not a huge fan on this suggestion, but curious what @chrisduerr has to say. It would be nice to find a way to be "in sync" with the launcher program, in one way or another. Meaning that all supported links were properly detected.

Either way, this is probably better handled as a separate issue, since this mode has been merged now, and the current functionality is working as intended.

@SRGOM
Copy link

SRGOM commented Mar 21, 2020

@nixpulvis That is indeed what I'm suggesting- basically say here's a sample shell session:

[~]
$ls /boot                                                                   
grub  kernels  lost+found  background.png  converted-font.pf2
[~]
$cat xx                                                                      
File: xx
http://google.com

file:///usr/bin/blahblah

https://blahblah.com


[~]

I would like to enter the "vi"-mode. Then move around and go wherever I want. Say I am on http://google.com. I don't know what's the shortcut for launching, but say it's go. (which in vi takes you a numeric parameter at the beginning, say a, and sends you to the ath byte in the buffer). So yeah I git go on http://google.com and alacrity sends xdg-open http://google.com. Then I continue moving around and focus on background.png and hit go again, then I get background.png opened in feh. Then I move left to kernels and hit go and ranger opens.

I don't know if you guys like it but if I feel this is both simple and intuitive (and while alacritty users are all power users, ultimately simplicity helps)

@nixpulvis
Copy link
Contributor

@SRGOM I recommend you try out this feature. Most of what you desire is already implemented.

If the VI mode cursor is over a valid URL, like the ones you've posted below cat xx, then pressing Enter ↵ will call the url.launcher, which defaults to xdg-open on Linux/BSD. However filenames like background.png aren't URLs so we don't currently support them.

@SRGOM
Copy link

SRGOM commented Mar 23, 2020

@nixpulvis Yeah basically my point was not specifically supporting anything, just blindly launching xdg-open so that I can also open background.png. Can't wait to try out the release after this though (I believe that's when the feature lands...)

@chrisduerr
Copy link
Member Author

@SRGOM I'm not a fan of adding a built in feature to blindly launch xdg-open, but I believe it's already possible to do what you want.

You can setup the key bindings so that whenever a binding is pressed, the active selection is copied to the clipboard and after that you can launch xdg-open using the data that has been saved to your clipboard. That should work just fine.

@nagy135
Copy link

nagy135 commented May 12, 2020

I think this is 90% of what I ever wanted from this, where last 10% would be search mode that was already mentioned as next feature by @chrisduerr. Amazing work, I m really excited about this ...in a meantime, tmux to the rescue

just curious, are you planning to support regex searches? if not that would be next level of said feature for sure !

kchibisov added a commit to kchibisov/alacritty that referenced this pull request Jun 14, 2020
Make notmodes follow modes in terms of override behavior, so
default binding won't be pruned if and only if its modes intersect
with new binding notmodes and vise-versa.

Fixes alacritty#3315.
@markstos
Copy link
Contributor

@chrisduerr Thanks for this feature. I use Tmux within Alacritty solely for its selection mode. Having this feature directly in Alacritty will simplify my life and save memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Modal keyboard controls
7 participants