search

hoosierhobbyist / quantum-shell

An experimental terminal emulator for the Atom text editor
MIT License
19 stars 6 forks source link

Failed to activate the quantum-shell package #9

Closed adrianhall closed 9 years ago

adrianhall commented 9 years ago

[Enter steps to reproduce below:]

  1. ...Install quantum shell on windows
  2. ...Toggle quantum shell

Atom Version: 0.190.0 System: Microsoft Windows 10 Pro Technical Preview Thrown From: quantum-shell package, v0.4.1

Stack Trace

Failed to activate the quantum-shell package

At Cannot read property 'split' of undefined

TypeError: Cannot read property 'split' of undefined
  at new QuantumShellModel (C:\Users\personal\.atom\packages\quantum-shell\lib\quantum-shell-model.coffee:50:30)
  at Object.module.exports.activate (C:\Users\personal\.atom\packages\quantum-shell\lib\quantum-shell-controller.coffee:28:22)
  at Package.module.exports.Package.activateNow (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\package.js:225:19)
  at C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\package.js:728:25
  at Emitter.module.exports.Emitter.emit (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\node_modules\event-kit\lib\emitter.js:82:11)
  at CommandRegistry.module.exports.CommandRegistry.handleCommandEvent (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:223:20)
  at CommandRegistry.handleCommandEvent (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:3:61)
  at CommandRegistry.module.exports.CommandRegistry.dispatch (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:157:19)
  at EventEmitter.<anonymous> (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\window-event-handler.js:73:30)
  at emitOne (events.js:77:13)
  at EventEmitter.emit (events.js:166:7)

Commands

     -0:14.4 application:add-project-folder (atom-text-editor.editor.is-focused)
     -0:01.5 quantum-shell:toggle (atom-text-editor.editor.is-focused)

Config

{
  "core": {
    "themes": [
      "atom-dark-ui",
      "atom-dark-syntax"
    ]
  }
}

Installed Packages

# User
autoclose-html, v0.15.0
autocomplete-haskell, v0.2.1
autocomplete-plus, v2.9.0
file-icons, v1.5.4
language-cshtml, v0.1.0
language-javascript-better, v1.3.0
omnisharp-atom, v0.2.1
quantum-shell, v0.4.1

# Dev
No dev packages
hoosierhobbyist commented 9 years ago

This is very strange. Can you tell me, does your environment have a PATH variable? It seems like you are using a very new/experimental version of windows with which I am unfamiliar. Also, quantum-shell support for any windows OS at this point is tentative at best. It is an eventual goal of mine, but unfortunately supporting all platforms is less than trivial. Thank you for the bug report though, I look forward to hearing back.

adrianhall commented 9 years ago

I'm using the Windows 10 Technical Preview build 10049 (which is the latest publically available). It does have a PATH variable.

C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Microsoft SQL Server\1
20\Tools\Binn\;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.4\;C:\Program Files (x86)\Git\cmd;C:\Program Files\Microsoft\Web Platform
 Installer\;C:\Program Files\Microsoft SQL Server\110\Tools\Binn\;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System
32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\nodejs\;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;C:\Users\pers
onal\.k\runtimes\kre-clr-win-x86.1.0.0-beta3\bin;C:\Users\personal\.k\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Window
s\System32\WindowsPowerShell\v1.0\;C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1
.4\;C:\Program Files (x86)\Git\cmd;C:\Program Files\Microsoft\Web Platform Installer\;C:\Program Files\Microsoft SQL Server\110\Tools\Binn\
;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\nodejs\;C:\Prog
ram Files (x86)\Windows Kits\10\Windows Performance Toolkit\;C:\Users\personal\AppData\Roaming\npm;C:\Program Files (x86)\Git\bin;C:\Users\
personal\AppData\Roaming\npm;C:\Users\personal\AppData\Local\atom\bin

I figured Windows support was probably marginal - it seems to be a common theme among all the Atom plugins except for those specifically designed for Windows.

hoosierhobbyist commented 9 years ago

If you don't mind, could you help me with a little debugging? I'm curious as to how this happened. Could you pull up the dev-tools (ctrl-shift-i) and run console.log(process.env.PATH)? For some reason, it doesn't seem to be visible within atom. Also, could you confirm that this is repeatable, and not just some weird one-time thing? I'd greatly appreciate it :smile:

adrianhall commented 9 years ago

It's ctrl-alt-I on Windows ;-) Yes - process.env.PATH contains the PATH as expected (and as listed above)

It is repeatable. I actually get two exceptions. The first one is listed in the bug, I also get the following:

[Enter steps to reproduce below:]

  1. ...
  2. ...

Atom Version: 0.190.0 System: Microsoft Windows 10 Pro Technical Preview Thrown From: quantum-shell package, v0.4.1

Stack Trace

Uncaught TypeError: Cannot read property 'isVisible' of null

At C:\Users\personal\.atom\packages\quantum-shell\lib\quantum-shell-controller.coffee:40

TypeError: Cannot read property 'isVisible' of null
  at Object.module.exports.toggle (C:\Users\personal\.atom\packages\quantum-shell\lib\quantum-shell-controller.coffee:40:18)
  at atom-workspace.<anonymous> (C:\Users\personal\.atom\packages\quantum-shell\lib\quantum-shell-controller.coffee:19:92)
  at CommandRegistry.module.exports.CommandRegistry.handleCommandEvent (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:242:29)
  at CommandRegistry.handleCommandEvent (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:3:61)
  at CommandRegistry.module.exports.CommandRegistry.dispatch (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\command-registry.js:157:19)
  at EventEmitter.<anonymous> (C:\Users\personal\AppData\Local\atom\app-0.190.0\resources\app\src\window-event-handler.js:73:30)
  at emitOne (events.js:77:13)
  at EventEmitter.emit (events.js:166:7)

Commands

     -1:06.4 dev-live-reload:reload-all (atom-text-editor.editor.is-focused)
     -0:58.9 window:toggle-dev-tools (atom-text-editor.editor.is-focused)
     -0:02.8 quantum-shell:toggle (atom-text-editor.editor.is-focused)

Config

{
  "core": {
    "themes": [
      "atom-dark-ui",
      "atom-dark-syntax"
    ]
  }
}

Installed Packages

# User
atom-terminal-panel, v4.3.1
autoclose-html, v0.15.0
autocomplete-plus, v2.9.0
file-icons, v1.5.4
language-cshtml, v0.1.0
language-javascript-better, v1.3.0
omnisharp-atom, v0.2.1
quantum-shell, v0.4.1
terminal-panel, v1.10.0
web-browser, v1.4.4

# Dev
No dev packages

I am not on the latest version, so I'll upgrade to 0.191.0 and let you know shortly if that fixed the issue.

adrianhall commented 9 years ago

... and this is also reproducible on version 0.191.0 of Atom.

hoosierhobbyist commented 9 years ago

Thank you for the help! This will require further investigation. That second error can safely be ignored however, as I am 99.9% certain it is a side effect of the first. I'll let you know if there's anything else I need, and hopefully get back to you in a day or two.

hoosierhobbyist commented 9 years ago

All right, so here's a little background for you: the code that is failing is directly related to the new tab-completion feature added in version 0.4.0. Basically, I am splitting the PATH variable on ':' (which I know is incorrect for windows) and caching a list of every executable file for use in the tab-completion command. If you look at the original stack trace it says:

TypeError: Cannot read property 'split' of undefined at new QuantumShellModel (C:\Users\personal.atom\packages\quantum-shell\lib\quantum-shell-model.coffee:50:30)

which as far as I can tell means that your environment's PATH is undefined (which is of course at odds with what you've already proven). See the code here.

A little more background: eventually I'd like to be able to support multiple terminals running simultaneously. In anticipation of that, I don't use process.env directly. Instead, each model either uses the serialized environment (from the last time quantum-shell was active) or, as in your case, uses a clone of process.env via underscore-plus like so: _.clone(process.env). See the code here.

What all of this means is there are two distinct possibilities (as far as I can tell). Either A) another package is undefining process.env.PATH (which seems unlikely) or B) there is a bug in underscore-plus that is failing to properly copy the PATH value of process.env. It could of course be something else entirely, but these seem like the two most likely explanations. To test A, I would like to ask that you manually disable every community package except for quantum-shell, reload atom, and see if this is still reproducible. If that doesn't reveal anything, we'll have to test possibility B. This would require you to clone quantum-shell locally, and checkout a special maintenance branch I would set up and push commits to that you could then test. If you are unsure of how to do that, I'd be happy to walk you through it. But before we worry about that, please do test explanation A and get back to me. And thank you for all that you've done/are doing to help with this, I truly appreciate it :smiley:

adrianhall commented 9 years ago

I installed a new virtual machine with Windows 10 Tech Preview, updated to the latest build and Atom build 0.191.0 on it (and nothing else), so this is about as virgin a test machine as we could hope for.

Side note - split the path on path.delimiter - https://nodejs.org/api/path.html#path_path_delimiter

I can confirm that the same issue is happening on a virgin machine.

I also opened up the Developer Tools and did a bit of testing: (output is modified for clarity)

> process.env.Path
"C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Users\photo\AppData\Local\atom\bin"

> var _ = require('underscore-plus');
> var x = _.clone(process.env)
> x.Path
"C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Users\photo\AppData\Local\atom\bin"

Happy to assist.

hoosierhobbyist commented 9 years ago

Thank you for that tip on path.delimiter, that will definitely make it into the next release. This may sound like a stupid question, but for the sake of a sanity check are you testing process.env.Path or process.env.PATH? If it's the first, could you check the all caps version? This could quite possibly explain the entire situation (I hope...if not I'm fresh out of ideas :worried:). Very nice debugging by the way, definitely easier than trying to coordinate me pushing commits to a maintenance branch you have cloned locally. I make things a little too complicated sometimes :sweat_smile:

adrianhall commented 9 years ago

You are now onto something ;-) I checked process.env.Path and process.env.PATH, then I cloned the process.env object and re-checked the clone. Results below.

> process.env.PATH
"C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Users\photo\AppData\Local\atom\bin"

> process.env.Path
"C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Users\photo\AppData\Local\atom\bin"

> var _ = require('underscore-plus');
> var x = _.clone(process.env);
> x.Path
"C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Users\photo\AppData\Local\atom\bin"

> x.PATH
undefined

I guess you can do something like 

var paths = (x.PATH || x.Path || "").split(path.delimiter);

to get either spelling?

hoosierhobbyist commented 9 years ago

Fantastic news! Your solution should do the trick quite nicely. I will get a patch out soon. In the mean time, do you think it would be appropriate to open up an issue with underscore-plus? This doesn't exactly strike me as expected behaviour.