romgrk / termrk

Terminal for atom, using pty.js & term.js
MIT License
33 stars 7 forks source link

Scrolling terminal throws an error #15

Closed ronaldocpontes closed 9 years ago

ronaldocpontes commented 9 years ago

[Enter steps to reproduce below:]

  1. run a command (e.g: npm install)
  2. use the mouse to scroll up
  3. the terminal screen will become all back and freeze

Atom Version: 0.202.0 System: Mac OS X 10.8.5 Thrown From: Termrk package, v0.1.13

Stack Trace

Uncaught TypeError: Cannot read property '0' of undefined

At /Users/ronaldo.pontes/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208

TypeError: Cannot read property '0' of undefined
  at Terminal.refresh (/Users/ronaldo.pontes/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208:18)
  at Terminal.scrollDisp (/Users/ronaldo.pontes/.atom/packages/Termrk/node_modules/term.js/src/term.js:1401:8)
  at TermrkView.module.exports.TermrkView.terminalMousewheel (/Users/ronaldo.pontes/.atom/packages/Termrk/lib/termrk-view.coffee:181:19)
  at TermrkView.terminalMousewheel (/Users/ronaldo.pontes/.atom/packages/Termrk/lib/termrk-view.coffee:2:1)

Commands

     -1:40.3.0 command-palette:toggle (atom-text-editor.editor.is-focused)
     -1:36.6.0 core:move-down (atom-text-editor.editor.mini.is-focused)
     -1:36.4.0 core:move-up (atom-text-editor.editor.mini.is-focused)
     -1:35.8.0 core:confirm (atom-text-editor.editor.mini.is-focused)
     -1:35.4.0 termrk:toggle (atom-text-editor.editor)

Config

{
  "core": {
    "themes": [
      "one-dark-ui",
      "atom-dark-syntax"
    ],
    "disabledPackages": [
      "local-history",
      "autoclose-html",
      "minimap"
    ]
  },
  "Termrk": {}
}

Installed Packages

# User
Termrk, v0.1.13
autocomplete-modules, v0.3.5
autocomplete-paths, v1.0.2
editorconfig, v1.0.0
highlight-selected, v0.9.3
jshint, v1.3.7
linter, v0.12.6
linter-csslint, v0.0.13
linter-flake8, v1.4.2
linter-htmlhint, v0.0.16
linter-jshint, v0.1.5
linter-pep8, v0.2.0
linter-tidy, v1.0.1
live-archive, v0.1.15
npm-autocomplete, v0.1.2
react, v0.11.8
recent-projects, v0.3.0

# Dev
No dev packages
romgrk commented 9 years ago

Well noted. This is probably caused by an experimental re-implementation of term.js::resize, because the original implementation delete lines where it shouldn't. It has been deactivated for now. I'll close but re-open if it happens again.

jsphstls commented 9 years ago

Just happened to me on 0.1.0.16

Opened Atom Opened Termrk with Alt+Space Entered some console commands Tried to scroll console Error appears and repeats

romgrk commented 9 years ago

Could you post the error log? TypeError is quite vague and could come from multiple places.

sigil66 commented 9 years ago

Log:

/Users/zachary/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208
Hide Stack Trace
TypeError: Cannot read property '0' of undefined
  at Terminal.refresh (/Users/zachary/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208:18)
  at Terminal.refresh (/Users/zachary/.atom/packages/Termrk/lib/termjs-fix.coffee:127:9)
  at Terminal.scrollDisp (/Users/zachary/.atom/packages/Termrk/node_modules/term.js/src/term.js:1401:8)
  at TermrkView.module.exports.TermrkView.terminalMousewheel (/Users/zachary/.atom/packages/Termrk/lib/termrk-view.coffee:163:19)
  at TermrkView.terminalMousewheel (/Users/zachary/.atom/packages/Termrk/lib/termrk-view.coffee:2:1)
isobit commented 9 years ago

Just got the same exact issue with 0.1.17:

/Users/joshglendenning/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208
TypeError: Cannot read property '0' of undefined
    at Terminal.refresh (/Users/joshglendenning/.atom/packages/Termrk/node_modules/term.js/src/term.js:1208:18)
    at Terminal.refresh (/Users/joshglendenning/.atom/packages/Termrk/lib/termjs-fix.coffee:127:41)
    at Terminal.scrollDisp (/Users/joshglendenning/.atom/packages/Termrk/node_modules/term.js/src/term.js:1401:8)
    at TermrkView.module.exports.TermrkView.terminalMousewheel (/Users/joshglendenning/.atom/packages/Termrk/lib/termrk-view.coffee:240:28)
    at TermrkView.terminalMousewheel (/Users/joshglendenning/.atom/packages/Termrk/lib/termrk-view.coffee:3:61)
ghost commented 9 years ago

Hello all,

I corrected this issue by modifing the terminalMousewheel event in lib/termk-view.coffee file, as follows:


terminalMousewheel: (event) =>
        deltaY  = event.wheelDeltaY
     #This line breaks things on OS X 
     #deltaY/120 is not needed for scrolling
        #deltaY /= 120
        deltaY *= -1
        @terminal.scrollDisp(deltaY)

From what I can tell, the way that the delta Y values from the "mouse wheel" are being used is causing the problem. Depending on your operating system, the way in which this value is interpreted varies. If this is the case, the way this mouse event is handled needs to be universal.

Mozilla has some good documentation talking about these delta values - https://developer.mozilla.org/en-US/docs/Web/Events/mousewheel

ghost commented 9 years ago

This solution works for me as well, does it work for anyone else? I changed how scrolling was handled -- I pretty much looked how term.js does it, and converted it over to CoffeeScript.


terminalMousewheel: (event) =>
        #This breaks everything :(
        #deltaY  = event.wheelDeltaY
        #deltaY /= 120
        #deltaY *= -1
        #Works on OSX only?
        #deltaY = event.wheelDeltaY
        #deltaY *= -1
        #Possible catch-all solution?
        deltaY = event.wheelDeltaY
        #Handle scrolling based on the event type
        #Based on how term.js handles mouse scrolling
        if event.type is 'DOMMouseScroll'
          deltaY += if event.detail < 0 then -1 else 1
          deltaY *= -1 #Correct scrolling direction
        else
          deltaY += if event.wheelDeltaY > 0 then -1 else 1
          deltaY *= -1 #Correct scrolling direction
        @terminal.scrollDisp(deltaY)

mjrodgers commented 9 years ago

Oooh yay! This fixes the bug for me on OS X! I'd recommend merging this!

ghost commented 9 years ago

@sasquatch666 Glad to hear!

I am looking into some other things to make a more solid fix for @romgrk -- Hopefully this is pushed as update shortly

:D

romgrk commented 9 years ago

Hello everyone,

So as a workaround, here's what's implemented:

Again, I cannot test the code on OS X, therefore I have to wait for your feedback; let me know how it goes. @ZachR0 if you see any changes that should be made, or have a more robust solution >> https://github.com/romgrk/termrk/blob/master/lib/termrk-view.coffee#L157 Don't hesitate to send a PR. (....or just the plain code, I'll do a good old copy/paste)

romgrk commented 9 years ago

This has been reported as fixed. Closing, but reopen if it's not the case.