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

zsh / oh-my-zsh - no scroll #151

Closed
normalser opened this issue Jun 28, 2016 · 29 comments · Fixed by #289
Closed

zsh / oh-my-zsh - no scroll #151

normalser opened this issue Jun 28, 2016 · 29 comments · Fixed by #289
Labels
type/bug Something is misbehaving

Comments

@normalser
Copy link

If i start demo with zsh instead of bash and have oh-my-zsh installed then the scroll will not work in xterm.js

I am not sure if this is an issue with xterm.js or oh-my-zsh

The command that causes scroll to stop working is this command: echoti smkx that is located in .oh-my-zsh/lib/key-bindings.zsh

One can also start xterm.js without oh-my-zsh and type that command into terminal and it will stop scrolling.

@parisk
Copy link
Contributor

parisk commented Jun 29, 2016

Thanks for reporting @wallverb. I do not have access to zsh right now, so I am not able to test this out.

Could you please take a look at your console in the developer tools of your browser and provide us with any errors/logs that might appear when attempting to scroll when in zsh with oh-my-zsh?

@normalser
Copy link
Author

@parisk - when zsh/oh-my-zsh is executing that echoti smkx command then the following data is being sent to Terminal.prototype.write = function(data):

�[?1l�>
xterm.js:1467 


xterm.js:1467 �]2;echoti smkx�
xterm.js:1467 �]1;echoti��[?1h�=�[1m�[7m%�[m�[1m�[m    

which seems like that = char is causing this:

 // ESC = Application Keypad (DECPAM).
              case '=':
                this.log('Serial port requested application keypad.');
                this.applicationKeypad = true;
                this.state = normal;
                break;

and then this.applicationKeypad = true; is causing return in scroll code:

      on(el, wheelEvent, function(ev) {
        if (self.mouseEvents) return;
        if (self.applicationKeypad) return;

@richarddavenport
Copy link

@parisk, any movement on fixing this? Really appreciate the work!

@parisk
Copy link
Contributor

parisk commented Jul 8, 2016

@richarddavenport unfortunately I didn't have time to deal with this and I do not project to be able to do so by the end of the month.

It would be extremely helpful if we could get an external contribution on this and I can definitely help anyone out with figuring out the internals of xterm.js.

@richarddavenport
Copy link

@parisk I'd love to help! Is there any documentation anywhere, or could you help me get going with debugging? Is there an editor or IDE that works best? VS Code/Webstorm/Chrome Dev Tools? Thanks!

@Tyriar
Copy link
Member

Tyriar commented Jul 11, 2016

@richarddavenport any editor works well. All the code is in ./src/xterm.js and instructions to run the demo are here.

@parisk
Copy link
Contributor

parisk commented Jul 11, 2016

@richarddavenport I think @Tyriar is right. Any editor supporting JavaScript works, so feel free to choose the one that suits you better.

Personal opinion though would be choosing an editor or IDE that uses xterm.js itself though, in order to dogfood it as well (use xterm.js when developing xterm.js). The two solutions that I am aware of right now are:

Debugging with Chrome/Firefox DevTools is the best way to go IMO.

Considering the documentation, there is no public website at the moment but you should expect it within the week. Till that time just run npm run build:docs in your terminal and then you will be able to browse the API reference in HTML format, inside the docs directory.

Feel free to ask for more help in case the above do not suffice.

@parisk
Copy link
Contributor

parisk commented Jul 11, 2016

@richarddavenport I published a preview version of the docs website, which contains the API reference. Most public methods are documented.

You can take a look at http://docs.xtermjs.org/. Hope it helps 😊 .

@richarddavenport
Copy link

Wow! That's pretty awesome! @wallverb is right about the echoti smkx command in oh my zsh. That command is putting the terminal in Application Mode, which (I'm guessing here) is related to the applicationKeypad property. Does that sound right? I have no idea what application mode is, but it sounds important. Line 1144, if commented out, everything works, but I'm unsure of the consequences or what that line is there for. Do you know what that is for?

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2016

Yes I'm a bit hesitant to touch it as I don't know what it does either.

This reference doc we use mentioned it here:

The application keypad transmits the following escape sequences depend-
ing on the mode specified via the DECKPNM and DECKPAM escape sequences.
Use the NumLock key to override the application mode.

The code also mentions:

              // ESC = Application Keypad (DECPAM).
              case '=':
                this.log('Serial port requested application keypad.');
                this.applicationKeypad = true;
                this.state = normal;
                break;

              // ESC > Normal Keypad (DECPNM).
              case '>':
                this.log('Switching back to normal keypad.');
                this.applicationKeypad = false;
                this.state = normal;
                break;

Looking into what DECPAM and DECPNM will likely lead us to an answer. Also if there's a difference with DECKPAM and DECKPNM.

@richarddavenport
Copy link

Xterm.js scrolling with oh my zsh now works, with this branch

@apjanke
Copy link

apjanke commented Jul 17, 2016

Hi. Found this thread from the related bug on Oh My Zsh.

Looking into what DECPAM and DECPNM will likely lead us to an answer. Also if there's a difference with DECKPAM and DECKPNM.

I think DECPAM and DECPNM are just typos for DECKPAM and DECKPNM. The mnemonics are "DEC KeyPad Application Mode" and "DEC KeyPad Normal Mode".

Application keypad modes change what character sequences the cursor keypad and/or numeric keypad keys send in response to key presses. There are two (or more) distinct keypad modes which can be set to either "application" or "normal": the numeric keypad and the cursor keypad. The applicationKeypad and applicationCursor properties modify the sequences sent by some keys, which seems appropriate.

It seems to me there may be a bug with the Home and End keys, though. Their behavior is modified by the applicationKeypad property. But xterm considers Home and End to be cursor keys, controlled by DECCKM instead of DECKPAM/DECKPNM. (See the "PC-Style Function Keys" section in that xterm reference document.) I think that means they should be controlled by applicationCursor instead of applicationKeypad.

Regardless, it's surprising to me that the keypad mode would affect mouse scroll wheel behavior. (That's the scrolling that OP is talking, about, right?) Maybe the fix is as simple as removing that if (self.applicationKeypad) return; check from the mouse scroll wheel handler.

I do not have access to zsh right now, so I am not able to test this out.

You can test this behavior under bash or other shells by doing tput smkx. That's equivalent to zsh's echoti. But be aware that under most terminfo definitions, smkx sends the sequences for both application cursor keypad and application numeric keypad modes, so you're not testing them independently if you do it that way. To test them independently (e.g. if you wanted to check which mode was controlling the Home/End behavior), you'd need to echo those escape sequences separately. (I don't remember how to do so off the top of my head.)

@Tyriar
Copy link
Member

Tyriar commented Jul 19, 2016

I believe this is the same root cause as why scrolling doesn't work in vim which also sets DECKPAM.

@jerch
Copy link
Member

jerch commented Jul 19, 2016

Following xterm's mouse spec the mouse actions don't get modified by DECKPAM or DECKPNM. Does anyone know why we have the if (self.applicationKeypad) return; in the mouse scroll handler at all?

@Tyriar
Copy link
Member

Tyriar commented Jul 19, 2016

@jerch yeah I'm beginning to think that too, there's a bunch of reading I want to do in #194 to finally fix these sorts of issues.

@parisk
Copy link
Contributor

parisk commented Jul 20, 2016

I think I have some more input to provide here.

Maybe the fix is as simple as removing that if (self.applicationKeypad) return; check from the mouse scroll wheel handler.
@apjanke this won't work and should not work actually. This particular event handler is there to allow scrolling in simple text mode applications like bash, which have applicationKeypad set to false (btw. sounds quite helpful to document this better).

In addition, even if we removed this it would result in a term.scrollDisp(1) call, which would do nothing when an app like vim is open, where term.ydisp and term.ybase are both 0.

This should be most probably handled in the previous event handler.

Xterm.js scrolling with oh my zsh now works, with this branch

@richarddavenport did this work without any change on xterm.js?

@parisk parisk added the type/bug Something is misbehaving label Jul 20, 2016
@richarddavenport
Copy link

@parisk Yes, it is working without any change to xterm.js.

@apjanke
Copy link

apjanke commented Jul 20, 2016

Maybe the fix is as simple as removing that if (self.applicationKeypad) return; check from the mouse scroll wheel handler.
@apjanke this won't work and should not work actually. This particular event handler is there to allow scrolling in simple text mode applications like bash, which have applicationKeypad set to false (btw. sounds quite helpful to document this better).

Shouldn't this behavior be just a function of the mouse tracking mode, Alternate Screen, and Alternate Scroll modes, and not the numeric/cursor Application Keypad modes? OP is using zsh, which is more like bash than like vim: it's a simple text application that uses the main screen and which does not provide its own scrolling support, so it relies on the terminal to do scrolling. But some of those simple text programs, especially zsh, might still enable Application Keypad mode so they can do portable key binding using terminfo definitions, which typically only include the application mode character sequences.

@jerch
Copy link
Member

jerch commented Jul 21, 2016

@parisk I second what apjanke says - why should a keyboard mode settings interfere at all with the mouse behavior? The official xterm spec does not mention any of this, therefore implementing an xterm compatible model may not have such a limitation. Maybe I miss something here.
"high level" terminal apps like midnight commander will register explicit mouse tracking - for those xterm just disables the default handler and the events are handled by the terminal app itself (which might lead to disabled scrolling - it is up to the app).

Furthermore the alternate screen switch (DECSET 1047) is used to indicate different terminal behavior - the normal screen buffer (1047 low) comes with the scrolling buffer while the alternate one (1047 high) has none. The latter is used by all curses apps, vim, emacs and other more sophisticated terminal apps using the terminal as a canvas (most apps heavily relying on terminfo). For those apps there is no terminal scrolling. You can test this yourself by running vim or emacs in xterm - you can't scroll back to previous terminal content. After closing the app and returning to normal shell interface terminal scrolling is "enabled" again.
Technically xterm maintains 2 separate screen buffers to achieve this. DECSET 1047 just switches between them.

There are also apps that do more advanced drawing on the normal screen (e.g. man, top) and don't register mouse tracking. xterm is still scrollable for those and their content just sits at the end of the output buffer.

@yankeeinlondon
Copy link

What's the easiest "work-around" available before this is addressed a more verbose manner?

@Tyriar
Copy link
Member

Tyriar commented Jul 29, 2016

@ksnyde this is probably the easiest: microsoft/vscode#7817 (comment). I haven't tried it personally though.

@apjanke
Copy link

apjanke commented Jul 29, 2016

this is probably the easiest: microsoft/vscode#7817 (comment). I haven't tried it personally though.

Unfortunately, that will break other key bindings which depend on zle_line_init. You may or may not use those key bindings, so it may or may not matter to you.

There's an open OMZ PR I put in to drop the zle_line_init/[sr]mkx dependency and stick with normal keypad mode, and which seems likely to be accepted. If you run that locally, or upvote it so it gets merged in to the OMZ mainline, you can use that as a workaround that avoids this scrolling issue (by avoiding application keypad mode altogether) and keeps all OMZ key bindings working.

That is, if you are using Oh My Zsh. If you are using one of the Linux distributions which configure zle_line_init to use [sr]mkx in their system config files, you'll need to redefine zle_line_init in your own ~/.zshrc config file as a do-nothing.

function zle-line-linit() {
  # NOP
}

(This may still break other key bindings that depend on application keypad mode.)

@apjanke
Copy link

apjanke commented Jul 29, 2016

For those apps there is no terminal scrolling. You can test this yourself by running vim or emacs in xterm - you can't scroll back to previous terminal content.

Minor difference, but this is not quite true: in xterm, at least some versions of it, if you run vim, it switches to the alternate screen, but you can still scroll back and see earlier contents of the buffer (as long as the buffer has more than a screen of content), as long as you have not enabled mouse event handling in vim. If you :set mouse=a in vim, then it will receive mouse events, and the terminal will not scroll in response to them. Most users probably don't see the former behavior because they have their .vimrc set up to automatically enable mouse event handling. I think this suggests that the terminal-level widget scrolling behavior is controlled by mouse event handling, not by the alternate screen mode.

@jerch
Copy link
Member

jerch commented Aug 8, 2016

Thanks for clarification @apjanke - to fully mimick xterm's behavior the handling of the scroll buffer and DECSET 1047 has to be reviewed first IMO.

@ayapi
Copy link
Contributor

ayapi commented Aug 20, 2016

@Tyriar , please teach me why 1b886a4 is needed.
you just want to hide a vertical scrollbar?
i observed xterm's behavior on linux for a little while, i thought that 1b886a4 should be dropped.

@Tyriar
Copy link
Member

Tyriar commented Sep 25, 2016

@ayapi that commit is being reverted in #287

That will make the scroll bar appear but the terminal won't react to the mouse wheel though.

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Sep 25, 2016
This change allows wheel events in application mode which fixes mouse wheel
scrolling in oh-my-zsh and powershell for Linux (among others). Along with xtermjs#287
a functional scroll bar will also be usable in those shells.

Fixes xtermjs#151
@apjanke
Copy link

apjanke commented Sep 25, 2016

FWIW, I think this is a case of Xterm.js deviating from normal xterm behavior, and is not Oh My Zsh's problem, and this bug should be closed. If OMZ does provide a compatibility shim for it, it should be confined to a plugin.

@Tyriar
Copy link
Member

Tyriar commented Sep 25, 2016

@apjanke so you agree with the PR I just put up? #289

@apjanke
Copy link

apjanke commented Sep 25, 2016

Yes, I agree with #289. "Application mode" as discussed here is just a keypad mode switch; mouse behavior should be unaffected by it, so wheel events should be allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants