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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to open files on Windows #1

Open
ScottFreeCode opened this issue Oct 1, 2016 · 11 comments
Open

Unable to open files on Windows #1

ScottFreeCode opened this issue Oct 1, 2016 · 11 comments
Labels

Comments

@ScottFreeCode
Copy link

The current version on NPM doesn't work on Windows due to: http://superuser.com/a/239572 This is handled in opener, to which master on GitHub delegates. Could we get a new NPM release? 馃樃

@nfischer
Copy link
Member

nfischer commented Oct 2, 2016

Sure thing! I'll take a look and update this. Thanks for notifying us

@nfischer
Copy link
Member

nfischer commented Oct 2, 2016

@ScottFreeCode Take a look at v0.1.2 馃槃 Please reopen this if the issue persists, and I'll gladly fix. Also, if you have any suggestions for the plugin, feel free to post a feature request.

@nfischer nfischer closed this as completed Oct 2, 2016
@ScottFreeCode
Copy link
Author

Hmm, well, the behavior of opening a console window is gone, but it also isn't really opening anything at all now. I'll dig into this and see if I can find out what's up. ;^) Thanks for the update in any case!

@nfischer
Copy link
Member

nfischer commented Oct 5, 2016

Hmmm... sounds strange. I thought I tested it on my windows box back in the day, but perhaps not.

If you figure out the issue, and can think of a way to make our unit tests and CI report more accurate results, let me know. I'll be more than happy to get this fixed.

@nfischer nfischer reopened this Oct 5, 2016
@ScottFreeCode
Copy link
Author

Unfortunately, I can't think of a way that opening a file could really be automatically tested for whether the file actually ends up opened... except for opening a shell script (effectively, running it, and shell scripts could write a file or something indicating success), but a quick test shows that that's the one case that's working when nothing else is (weird, huh).

I have, however, determined that it's somehow caused by calling unref on the returned process too soon (calling unref as soon as possible on the process's std streams seems to be fine though). Might be a Node issue, or might be an issue with the way Opener is configuring it, or might just be a usage issue (in which case, given that the usage in this plugin matches the documentation for Opener, that documentation would need to be updated too).

A possible usage workaround might be moving the call to proc.unref into a callback in process.on('exit',...), but it's unclear to me from the Node documentation whether the process's exit event will wait till after non-unrefed child processes finish (in which case this would be useless) or not.

@nfischer
Copy link
Member

nfischer commented Oct 6, 2016

@ScottFreeCode Would a few milliseconds help? We could add a timeout to unref it after a few milliseconds. It won't affect CPU usage, and it'll only slow down the function by a little bit. If it's just a race condition, this might be sufficient to avoid it (although it's definitely a bit of a hack).

@ScottFreeCode
Copy link
Author

Something like that should work around it in general (I tried with a one-second delay to verify that it's a timing issue), I just don't know how long it needs to be or whether it depends on how fast Windows is doing whatever it's doing (and by extension how bogged down the OS is at the moment when it's run).

I'm thinking of working out how to test the behavior using the underlying Node commands and opening a ticket with Node to see if we can find out what to do, for what it's worth.

@nfischer
Copy link
Member

nfischer commented Oct 6, 2016

Sounds good. Let's wait on a fix here until we figure out if this is a Node problem. Once we know the real problem, we can either fix up at Node or do a workaround here.

Would you be able to provide an example command that fails? And it's only on Windows from what you've observed, right?

@nfischer
Copy link
Member

@ScottFreeCode I just tried this out on my windows machine and saw no problems. Here's what I tried:

> require('shelljs-plugin-open');
> var shell = require('shelljs');
> shell.open('https://www.google.com'); // opens my web browser
> shell.open('.'); // opens the current folder

Trying to open some files can fail, but that's only because Windows either isn't using the correct program for the filetype (.js fails for some reason), or it doesn't know which program to use (.md is a great example of this).

I suppose this means we should probably change our tests to only open well-recognized filetypes, like .txt. But aside from that, I'm not sure what issue you're having, or how I can proceed with fixing it.

@ScottFreeCode
Copy link
Author

Hmm, well, I got the issue opening .txt files and .url files, and Windows doesn't report any error -- just, nothing happens. I've come up with a pretty simple way to reproduce the underlying issue though:

test.js

var proc = require('child_process').execFile('cmd', ['/c', 'start', '', process.argv[2]])
proc.unref()
proc.stdin.unref()
proc.stdout.unref()
proc.stderr.unref()

commandline

node test.js test.txt

Seems like if I comment out any of the unref calls it starts working, but other experiments suggest that those unref calls are in general necessary to avoid waiting for some programs to finish (depending on the type of file opened).

@nfischer
Copy link
Member

Ok, I'll try to take a look at reproducing when I get the opportunity. Thanks

@nfischer nfischer self-assigned this Jun 7, 2017
@nfischer nfischer changed the title update npm release Unable to open files on Windows Oct 31, 2017
@nfischer nfischer removed their assignment Jul 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants