Skip to content
This repository has been archived by the owner on Jan 14, 2022. It is now read-only.

Send xterm mouse tracking codes #256

Merged
merged 14 commits into from Jul 21, 2013
Merged

Send xterm mouse tracking codes #256

merged 14 commits into from Jul 21, 2013

Conversation

dstahlke
Copy link
Contributor

@dstahlke dstahlke commented Jul 7, 2013

This patch provides an option to send xterm mouse tracking codes to the terminal. When the option is enabled, and when the console application requests mouse tracking, tap events are forwarded as clicks and drag/fling events are forwarded as scroll wheel events. Examples of applications which can be configured to accept mouse events are vim and tmux.

Currently less does not scroll. Terminals such as gnome-terminal and konsole forward scroll events as cursor keys when applications are using the alternate screen buffer. If the present patch is accepted, I plan to next implement this kludge so that less can scroll as well.

I have not implemented the DEC save and restore commands for the mouse tracking flags, although this probably doesn't matter. Probably the way forward here is to use a BitSet rather than an integer to store the flags, as this will enable handling flags greater than 32 in a unified way.

@jackpal
Copy link
Owner

jackpal commented Jul 8, 2013

Neat! It may take me a few days to read all this, but sounds good!

jackpal added a commit that referenced this pull request Jul 21, 2013
Send xterm mouse tracking codes
@jackpal jackpal merged commit 2ce1914 into jackpal:master Jul 21, 2013
@jackpal
Copy link
Owner

jackpal commented Jul 21, 2013

Looks good!

-- The code that converts screen coordinates into character coordinates should guard against overflowing bytes. Seems like there should be some Math.min(191,xxx) statements in a few places.

@dstahlke
Copy link
Contributor Author

I agree with using Math.min(191, xxx). Should I change this or will you? I believe there is a new type of mouse mode that is capable of sending coordinates >191, but I don't know the current level of support for this in programs or terminals.

As for your other comments, they appear to be in reference to intermediate versions of the code, not the final version that I intended. For example, the commented out code you mentioned was only commented out as an experiment and the changes to pushAndRun were only for personal use and I reverted them before submitting the pull request. So why were these changes even visible as you were evaluating the pull request? Did I do something wrong with git? I am a bit inexperienced with sending pull requests, so your help in clearing up this matter will be appreciated.

@jackpal
Copy link
Owner

jackpal commented Jul 21, 2013

Hi Dan,

Yes, please make the Math.min() change yourself. Ideally as a stand-alone
pull request, but I'd also be happy to see it as part of a CL or a series
of CLs related to improving the behavior of the "less" command.

The way git works, when you create a pull request, git will send each of
the changes that you checked in.

For this CL I started reviewing your checkins one-by-one, then realized
that you hadn't intended them to be reviewed individually. So I switched to
reviewing the whole set of changes as if it had been one CL. Sorry for
leaving my comments on your first CL. I should have removed my comments to
avoid confusion.

In the future you might want to look into the "git rebase" command. This
lets you edit your commit history. Before sending out a CL, you could use
git rebase to "squash" all your CLs down into a single commit, or
alternately you could create a series of small easy-to-test-and-review CLs.

Some people send me one commit for each feature they implement, so I can
accept or reject the features individually. (That's handy when we're
testing too -- we can drop out just one CL and see if the bug goes away.)

It's tricky to learn how to rebase commits, but once you know how, it is
very handy.

Note that you should only use git rebase on ranges of CLs that you haven't
"git push"ed back to github. If you rebase CLs that have already been
published, and then publish them again, it can cause extra work for other
people who have already copied part of your changes into your tree.

On Sun, Jul 21, 2013 at 2:22 PM, Dan Stahlke notifications@github.comwrote:

I agree with using Math.min(191, xxx). Should I change this or will you?
I believe there is a new type of mouse mode that is capable of sending
coordinates >191, but I don't know the current level of support for this in
programs or terminals.

As for your other comments, they appear to be in reference to intermediate
versions of the code, not the final version that I intended. For example,
the commented out code you mentioned was only commented out as an
experiment and the changes to pushAndRun were only for personal use and I
reverted them before submitting the pull request. So why were these changes
even visible as you were evaluating the pull request? Did I do something
wrong with git? I am a bit inexperienced with sending pull requests, so
your help in clearing up this matter will be appreciated.


Reply to this email directly or view it on GitHubhttps://github.com//pull/256#issuecomment-21317301
.

@dstahlke
Copy link
Contributor Author

Thank you for taking the time to write this very informative reply. I will make sure to use rebase in the future.

Shortly I will send a pull request for the Math.min change. Or perhaps it would be better to just ignore presses outside of the defined range. I'll give it a few minutes thought.

As for the less command, I have decided that it probably isn't best to put any hacks into the terminal emulator to send arrow keys to get less to scroll, even if konsole and gnome-terminal do this. My reasoning is that 1) as far as I can tell less is the only often used command that would benefit, 2) there already is a patch for less that enables it to properly interface with the mouse (I tested this and it works well), so hopefully this will make its way into the main codebase, 3) sending arrow presses can cause unexpected behavior in some cases like in readline under tmux or screen.

However, I am occasionally thinking of other ways the touchscreen could be used, such as for repositioning the cursor on the command line, or useful options such as making the bottom line easier to click (useful for switching between windows in tmux). So maybe I will send some more pull requests when I get more free time.

@dstahlke dstahlke deleted the scrolling branch July 22, 2013 00:06
@dstahlke
Copy link
Contributor Author

A question before I do the clipping fix: you say there may be overflow for x >= 192. But 192+32=224. Wouldn't overflow only occur for x > 255-32 = 223?

@jackpal
Copy link
Owner

jackpal commented Jul 22, 2013

Heh, yes your math is right. I subtracted wrong.
On Jul 21, 2013 5:14 PM, "Dan Stahlke" notifications@github.com wrote:

A question before I do the clipping fix: you say there may be overflow for
x >= 192. But 192+32=224. Wouldn't overflow only occur for x > 255-32 = 223?


Reply to this email directly or view it on GitHubhttps://github.com//pull/256#issuecomment-21319810
.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants