Enable shift+select in mouse mode #974

chrisduerr · 2017-12-28T20:37:52Z

When an application takes control over the mouse, it usually disables
selection completely. However the common way to still make selection
possible is by allowing selection while the shift key is held down.

This feature is implemented here by making use of the new modifiers
field on mouse events with glutin/winit.

This fixes #146.

I've thought about changing the cursor, but it seems like that would be kinda odd (you wouldn't want it changing every time when hitting shift). And other terminal emulators also do not seem to change the cursor with shift selection.
For clearing a selection you also need to hold shift and click, but this also seems consistent with my experience on other terminal emulators.
If you have any feedback on these two things, feel free to tell me about it.

maximbaz · 2017-12-28T22:52:12Z

Thanks, works nicely. I find it a bit odd that I have to hold Shift when clicking to clear the selection, even if other terminals do the same - I can't imagine a scenario where I would shift-select something, then start clicking without shift and expecting the selection to stay.

chrisduerr · 2017-12-28T23:14:32Z

Yeah I have definitely thought about just setting it up so clicking always clears selection no matter if shift is held or not. However until now alacritty also hasn't cleared when the mouse modes were enabled so I decided to stick with this for now. If anyone has a reason why the selection shouldn't be cleared when mouse mode is enabled and left mouse is clicked, please tell me.

Otherwise I'd be inclined to change it to always clear on left click. Maybe @jwilm knows more about this kind of stuff? I'd say the current state of the PR is the "safe" route, so I'm gonna stick with it for now.

Also while termite doesn't clear the selection when clicking without shift, it also doesn't keep up the selection when the buffer changes. So when I change rooms in weechat with a left click after selecting something, the selection is cleared. Currently with alacritty the selection would stay and effectively something completely different would be selected.

unphased · 2017-12-29T06:01:27Z

I have no preference and have found myself eventually mashing Esc to clear it when I couldn't figure it out. I could say that the terms I use (mostly iterm) do clear it on the next regular click...

One thing I wanted to bring up is that I get really good use out of Shift+Alt+drag for selecting a block. Consider that a feature request. :)

This is very useful in vim/tmux to omit line numbers and other vertically split panes while copying code or log output, for example. Indeed I use block select at least 80% of the time. The reason why block select isn't necessarily an essential feature for me is that zooming to maximize the pane with tmux lets me use regular select in a pinch.

jwilm · 2017-12-29T16:47:44Z

How does this compare with @gilbertw1's implementation?

chrisduerr · 2017-12-29T17:01:01Z

I did not see an open PR from him. But looking at his master it looks like the main difference is that he's passing only the shift mod and I'm passing all modifiers. And mine is not using the git version of glutin.

If you want to wait from a PR of him and merge that, feel free. I don't really care as long as this feature is merged. There are probably a few things that could be improved in both implementations, but they look almost identical.

gilbertw1 · 2017-12-29T17:39:59Z

I hadn't opened a PR for it yet. As @chrisduerr said this implementation does look nearly identical to my fork with the exception that he's passing through all of the modifiers, which I believe is probably a better choice.

Looks good to me 👍

Edit: The only difference behavior wise with mine is that single clicking does clear out the selection, unless the mouse input is captured by an application, in which case only shift + click will clear the selection.

chrisduerr · 2017-12-29T17:52:10Z

The single-click behavior seems to be the same for our versions from what I can tell. I've updated this PR to use only the shift key.

Now they seem to be pretty much identical, except for the test which @gilbertw1 didn't change.

maximbaz · 2017-12-29T21:13:15Z

So is it settled that only Shift+Click clears the selection, and not the Click alone? Still find that odd 😛

chrisduerr · 2017-12-29T21:33:51Z

Based on the current implementation that would be the "safe" choice. But I still am open to change this. Maybe @jwilm has an opinion on that?

I don't think it's a big deal either way, but I'd love to get some more input on this.

DSpeckhals · 2018-01-02T14:06:05Z

I prefer a simple click clearing the selection, but I'm not completely opposed to shift-click. This is a great feature overall!

chrisduerr · 2018-01-02T16:06:02Z

It seems like the common consensus is that even in mouse mode, the
selection should be cleared with a normal click, so I've changed the behavior to that.

If there is any reason why this should not be the case, please let me
know.

Note that in the current behavior of master, the selection would not be cleared when the mouse is captured by an application. However I don't know of any way to make a selection in an application that captures the mouse without this PR.

When an application takes control over the mouse, it usually disables selection completely. However the common way to still make selection possible is by allowing selection while the shift key is held down. This feature is implemented here by making use of the new `modifiers` field on mouse events with glutin/winit. This fixes alacritty#146.

The only mouse modifier required right now is the shift key, to prevent passing around unnecessary state, only the shift state is passed to the mouse processors now.

Three was still a test which passed the whole modifiers struct instead of just the shift bool, this has been fixed.

The tests were using a manually setup `ModifiersState`, to clean things up a bit the `ModifiersState::default` method has been used to replace this.

It seems like the common consensus is that even in mouse mode, the selection should be cleared with a normal click. If there is any reason why this should not be the case, please let me know.

unphased · 2018-01-02T23:53:25Z

I noticed that doubleclicking with this makes the selection work word-wise. Very good!

chrisduerr · 2018-01-02T23:57:23Z

Double-clicking in general is used for semantic selection. And triple-clicking is used for line-selection. This PR didn't introduce this but the functionality should work when holding shift in mouse-mode now.

unphased · 2018-01-03T00:05:39Z

Ah yes I remember it working from before, too. Being in tmux most of the time I had forgotten.

unphased · 2018-01-03T00:08:41Z

BTW, I am used to how selecting copies what was selected to the system buffer right away. I wonder if a configuration can be added to make this happen? Uh, I can make a feature request for this.

chrisduerr · 2018-01-03T00:15:44Z

This should be the case for both normal selection and selection with shift when mouse-mode is enabled. Is this not working for you?

maximbaz · 2018-01-03T01:14:13Z

It is implemented, just keep in mind that mouse selection puts contents to the "primary selection clipboard" (something you would paste in vim with "*p), while pressing Ctrl+Shift+C on a selection puts contents to the "system clipboard" (something you would paste in vim with "+p). Maybe this was the confusion? 🙂

chrisduerr · 2018-01-03T02:55:44Z

I think he's talking about MacOS, not linux. I've talked to jwilm on IRC and he said there's no equivalent functionality in other macOS terminals, so directly copying to clipboard is not implemented in MacOS.

If there is a convention how this should be done (I think @unphased has mentioned that iTerm2 supports this), then implementing this feature for MacOS too should be possible.

jwilm

Thanks for the PR @chrisduerr! Excited to land this feature which people have been asking for as long as I can remember.

Everything looks good sans one minor change.

jwilm · 2018-01-03T16:34:32Z

src/input.rs

-                if let Event::WindowEvent { event: WindowEvent::MouseInput { state, button, .. }, .. } = $input {
-                    processor.mouse_input(state, button);
+                if let Event::WindowEvent { event: WindowEvent::MouseInput { state, button, modifiers, .. }, .. } = $input {
+                    processor.mouse_input(state, button, modifiers.shift);


It seems to me that modifiers should be passed through in its entirety. If we ever want to add some other mouse + modifier functionality, then all the mods are already being passed without bloating the method signature.

ModifiersState is now passed to the mouse methods in `input.rs` as a whole instead of just passing the `shift` state. This should make it a bit easier to do changes in the future.

unphased · 2018-01-03T18:37:51Z

Thanks for the clarifications guys!

lf94 · 2018-01-03T23:01:56Z

Am I supposed to be able to hold shift + mouse drag to select text in tmux now? Because it isn't working after a git pull and cargo build --release && cargo install :(

chrisduerr · 2018-01-03T23:20:02Z

@lf94 Are you sure you are on the latest master? Usually in tmux you should be able to just select text by default as long as mouse mode is not enabled. After enabling mouse mode, you should now be able to select text with shift + mouse.

At least this is the way it works on my X11 machine. Are you on macos?

lf94 · 2018-01-03T23:21:54Z

7b4ba80 is the first bit of the latest commit hash.

I'm on Debian Stretch (9.0).

I have mouse mode enabled in tmux, as it's always been :) (set -g mouse on)

tmux -V reports 2.3

chrisduerr · 2018-01-03T23:27:18Z

I'm not quite sure what to tell you then. It seems to work just fine with tmux 2.6 on archlinux.

What exactly happens when you try to shift-select? Do you still get the mouse selection from tmux?

lf94 · 2018-01-03T23:28:02Z

Yeah that is exactly what is happening still.

lf94 · 2018-01-03T23:29:58Z

My configuration:

set -g mouse on
set-option -g status-position top
set -g default-terminal "screen-256color"

## COLORSCHEME: gruvbox dark
set-option -g status "on"

# default statusbar colors
set-option -g status-bg default #bg1
set-option -g status-fg default #fg1

# default window title colors
set-window-option -g window-status-bg default #yellow
set-window-option -g window-status-fg default #bg1

set-window-option -g window-status-activity-bg default #bg1
set-window-option -g window-status-activity-fg default #fg3

# active window title colors
set-window-option -g window-status-current-bg default
set-window-option -g window-status-current-fg default #bg1

# pane border
set-option -g pane-active-border-fg default #fg2
set-option -g pane-border-fg default #bg1

# pane number display
set-option -g display-panes-active-colour default #fg2
set-option -g display-panes-colour default #bg1


set-option -g status-attr "underscore"
set-option -g status-justify "left"
set-option -g status-left-attr "underscore"
set-option -g status-left-length "0"
set-option -g status-right-attr "underscore"
set-option -g status-right-length "0"
set-window-option -g window-status-activity-attr "none"
set-window-option -g window-status-attr "none"
set-window-option -g window-status-separator ""

set-option -g status-left ""
set-option -g status-right ""
set-window-option -g window-status-current-format "#[fg=default, bg=colour136, bold] #I #W "
set-window-option -g window-status-format "#[fg=default, bg=default] #I #W "

chrisduerr · 2018-01-03T23:39:31Z

I'm unable to reproduce even with your config.

maximbaz · 2018-01-03T23:41:01Z

@lf94 is it possible that you use an unusual keyboard layout maybe? For the record, I can't reproduce the issue too.

lf94 · 2018-01-03T23:44:00Z

I'm using Dvorak, setup via Cinnamon's configuration tools.

It works in xterm/urxvt/gnome-terminal though just fine.

lf94 · 2018-01-03T23:44:41Z

Is there a way to verify my alacritty binary? You guys have a SHA-1 we can compare?

lf94 · 2018-01-03T23:46:01Z

0edb8ba861... is the first bit of the hash.

(I am not editing my posts in case lurkers are watching via email.)

maximbaz · 2018-01-03T23:52:12Z

I wonder if this is somehow related or caused by #61, specifically:

There's an issue supporting alternative keyboard layouts with Glutin's input system...

To double check, try QWERTY maybe?

My binary, compiled on Arch Linux: alacritty.zip

chrisduerr · 2018-01-03T23:53:29Z

If cat src/input.rs | grep modifiers.shift gives you any results, you shoud definitely have the feature.
Also try executing it with cargo run (--release) directly.

Dvorak should not be an issue, I'm using it myself and it works just fine.

lf94 · 2018-01-04T00:08:44Z

Yes, it gives me results.

I'm now waiting for it to compile with cargo run --release.

Also, our hashes did not equal each other. Hm.

lf94 · 2018-01-04T00:22:26Z

cargo run --release worked.

It seems cargo install --force didn't work properly!

Mission complete. Thank you guys :)

chrisduerr · 2018-01-04T01:25:32Z

No problem. :)

chrisduerr mentioned this pull request Dec 29, 2017

Block selection #978

Closed

chrisduerr mentioned this pull request Dec 29, 2017

Bump glutin to 0.12 #979

Closed

maximbaz mentioned this pull request Dec 31, 2017

Implement block selection #981

Closed

chrisduerr added 5 commits January 2, 2018 17:28

Update mouse modifiers to only pass shift

82d64f8

The only mouse modifier required right now is the shift key, to prevent passing around unnecessary state, only the shift state is passed to the mouse processors now.

Fix failed tests

f2d7ee9

Three was still a test which passed the whole modifiers struct instead of just the shift bool, this has been fixed.

Use default modifier state

61caf87

The tests were using a manually setup `ModifiersState`, to clean things up a bit the `ModifiersState::default` method has been used to replace this.

Always clear selection

6525d05

It seems like the common consensus is that even in mouse mode, the selection should be cleared with a normal click. If there is any reason why this should not be the case, please let me know.

chrisduerr force-pushed the shift-select branch from b329698 to 6525d05 Compare January 2, 2018 16:29

jwilm requested changes Jan 3, 2018

View reviewed changes

pklingem mentioned this pull request Jan 3, 2018

Library libwayland-egl.so could not be loaded. #984

Closed

Pass ModifiersState to mouse input as whole

13162db

ModifiersState is now passed to the mouse methods in `input.rs` as a whole instead of just passing the `shift` state. This should make it a bit easier to do changes in the future.

Pass ModifiersState to mouse_moved method too

9f11464

jwilm approved these changes Jan 3, 2018

View reviewed changes

jwilm merged commit 7b4ba80 into alacritty:master Jan 3, 2018

chrisduerr deleted the shift-select branch January 3, 2018 19:21

chrisduerr restored the shift-select branch January 3, 2018 19:22

chrisduerr deleted the shift-select branch January 3, 2018 19:50

unphased mentioned this pull request Mar 9, 2018

Region Selection + Clipboard Copy #6

Closed

Enable shift+select in mouse mode #974

Enable shift+select in mouse mode #974

Conversation

chrisduerr commented Dec 28, 2017 • edited

maximbaz commented Dec 28, 2017

chrisduerr commented Dec 28, 2017 • edited

unphased commented Dec 29, 2017 • edited

jwilm commented Dec 29, 2017

chrisduerr commented Dec 29, 2017

gilbertw1 commented Dec 29, 2017 • edited

chrisduerr commented Dec 29, 2017

maximbaz commented Dec 29, 2017

chrisduerr commented Dec 29, 2017

DSpeckhals commented Jan 2, 2018

chrisduerr commented Jan 2, 2018 • edited

unphased commented Jan 2, 2018

chrisduerr commented Jan 2, 2018

unphased commented Jan 3, 2018

unphased commented Jan 3, 2018 • edited

chrisduerr commented Jan 3, 2018

maximbaz commented Jan 3, 2018

chrisduerr commented Jan 3, 2018

jwilm left a comment

Choose a reason for hiding this comment

jwilm Jan 3, 2018

Choose a reason for hiding this comment

unphased commented Jan 3, 2018

lf94 commented Jan 3, 2018

chrisduerr commented Jan 3, 2018

lf94 commented Jan 3, 2018 • edited

chrisduerr commented Jan 3, 2018

lf94 commented Jan 3, 2018

lf94 commented Jan 3, 2018

chrisduerr commented Jan 3, 2018

maximbaz commented Jan 3, 2018

lf94 commented Jan 3, 2018

lf94 commented Jan 3, 2018

lf94 commented Jan 3, 2018 • edited

maximbaz commented Jan 3, 2018

chrisduerr commented Jan 3, 2018

lf94 commented Jan 4, 2018 • edited

lf94 commented Jan 4, 2018

chrisduerr commented Jan 4, 2018

chrisduerr commented Dec 28, 2017 •

edited

chrisduerr commented Dec 28, 2017 •

edited

unphased commented Dec 29, 2017 •

edited

gilbertw1 commented Dec 29, 2017 •

edited

chrisduerr commented Jan 2, 2018 •

edited

unphased commented Jan 3, 2018 •

edited

lf94 commented Jan 3, 2018 •

edited

lf94 commented Jan 3, 2018 •

edited

lf94 commented Jan 4, 2018 •

edited