Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

process.stdout "resize" events broken on OSX in v0.10.29 #7855

Closed
jpillora opened this issue Jun 28, 2014 · 3 comments
Closed

process.stdout "resize" events broken on OSX in v0.10.29 #7855

jpillora opened this issue Jun 28, 2014 · 3 comments

Comments

@jpillora
Copy link

In the recent v0.10.29 update under OS X (10.9.3), the following does not work as expected:

//keep terminal open
process.stdin.on('data', function() {
  process.exit(1);
});
//bind listener
process.stdout.on('resize', function() {
  var width = process.stdout.columns;
  var height = process.stdout.rows;
  console.log('%sx%s', width, height);
});
//... do some resizing ... and nothing...

Whereas it works in 0.10.28+OSX and works in 0.10.29+Ubuntu

root@ubuntu:~# node resize.js
108x14
108x15
108x16

@TooTallNate thoughts?

Edit 1: Clarified which Node version

Edit 2: Upon further inspection, process.stdout.columns and process.stdout.rows are also not reflecting the current terminal size:

setInterval(function(){
  var width = process.stdout.columns;
  var height = process.stdout.rows;
  console.log('%sx%s', width, height);
}, 1000);

Edit 3: process.stdout.getWindowSize() also does not update

@jpillora jpillora changed the title process.stdout "resize" events broken on OSX process.stdout "resize" events broken on OSX in v0.10.29 Jun 28, 2014
@cjihrig
Copy link

cjihrig commented Jun 28, 2014

The problem appears to be in /src/node_constants.cc with this line that includes signal.h. If this line is moved above line 27 (the _XOPEN_SOURCE code), then everything works as expected. This was the case as recently as v0.11.13.

I would make the PR to move the #include, but it looks like @indutny and @bnoordhuis have had some back and forth in 00890e4 and 885142a on how that code should be organized.

@bnoordhuis
Copy link
Member

The definition of _XOPEN_SOURCE seems to hide SIGWINCH on OS X. I suppose that's reasonable, it's not a standard signal.

It can be worked around by defining _DARWIN_C_SOURCE but that's arguably bolting a hack on top of a hack.

indutny added a commit to indutny/node that referenced this issue Jun 30, 2014
It appears that it is defined unconditionally on all supported unixes.

fix nodejs#7867 nodejs#7855
indutny added a commit that referenced this issue Jul 2, 2014
It appears that it is defined unconditionally on all supported unixes.

fix #7867 #7855

Signed-off-by: Trevor Norris <trev.norris@gmail.com>
@trevnorris
Copy link

Supposed to've been fixed by 9cbfd6e.

mscdex pushed a commit to mscdex/node that referenced this issue Dec 25, 2014
It appears that it is defined unconditionally on all supported unixes.

fix nodejs#7867 nodejs#7855

Signed-off-by: Trevor Norris <trev.norris@gmail.com>
abernix pushed a commit to meteor/node-legacy that referenced this issue Sep 28, 2017
* **crypto**
  * Support for multiple ECDH curves. [nodejs#15206](nodejs/node#15206)
* **dgram**
  * Added `setMulticastInterface()` API. [nodejs#7855](nodejs/node#7855)
  * Custom lookup functions are now supported. [nodejs#14560](nodejs/node#14560)
* **n-api**
  * The command-line flag is no longer required to use N-API. [nodejs#14902](nodejs/node#14902)
* **tls**
  * Docs-only deprecation of `parseCertString()`. [nodejs#14245](nodejs/node#14245)
* **New Contributors**
  * Welcome Sebastiaan Deckers (@sebdeckers) as a new Collaborator! [nodejs#15354](nodejs/node#15354)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants