sindresorhus / grunt-shell

Run shell commands
MIT License
949 stars 126 forks source link

Enable stdin raw mode for tty processes #74

Closed taavo closed 10 years ago

taavo commented 10 years ago

When working on a package, I often use a bash script to open a repl with the package I'm working on pre-loaded. Something like this:

#!/usr/bin/env node
var repl = require('repl');

myPackage = require('../myPackage.js');

var r = repl.start({
  terminal: true
});

r.on('exit', function() {
  process.exit();
});

This works great, but I'd like to use grunt-shell so I can call my console with grunt console instead of bin/console.js. The issue is that unless raw mode is enabled, vt100 escape codes aren't passed to the child process, and crucial repl features like history no longer work.

This pull request simply enables raw mode for process.stdin.

Thanks!

sindresorhus commented 10 years ago

Is there any downside turning this on? Will it break for something?

taavo commented 10 years ago

The docs on raw mode are pretty thin, but as far as I can tell the difference is that by default stdio is buffered a line at a time, but in raw mode each keypress is sent individually. I'll be doing some more testing today, but if you're still unsure it could be worth checking with other people using grunt shell with interactive commands, like @gingermusketeer.

Worst case scenario I'll resubmit with a "raw" option or something like that. Thanks.

On Aug 17, 2014, at 7:23 PM, Sindre Sorhus notifications@github.com wrote:

Is there any downside turning this on? Will it break for something?

— Reply to this email directly or view it on GitHub.

taavo commented 10 years ago

Given the haziness of the docs, I decided to add rawMode as an option. The default is true, since I haven't yet seen a scenario within grunt-shell where that wasn't an improvement. Let me know if you have any further thoughts.

sindresorhus commented 10 years ago

I don't want an option. Let's just go with it by default.

taavo commented 10 years ago

Roger that—I've reverted the PR to just always set rawMode to true again. We can always add an option if someone reports an issue.

falaa commented 10 years ago

Hi, unfortunately rawMode breaks code running with pseudoTTY in docker. Please add option for switching it off.

mgol commented 10 years ago

I'd add to what @falaa said that since this is the second time sth breaks because of this option (a separate case was reported in a PR #75) in a week or so and our cases seem much more common than @taavo's one (most people don't update packages very often so most people weren't hit by this change yet) not only the option should be added but it should also be set to false by default.

mgol commented 10 years ago

To not pollute this thread more, I created issue #78.