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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed hang when called with no args or STDIN piped data #179

Merged
merged 2 commits into from
Jul 30, 2016
Merged

Fixed hang when called with no args or STDIN piped data #179

merged 2 commits into from
Jul 30, 2016

Conversation

luizberti
Copy link
Contributor

Will check if STDIN is associated with an interactive fd using isatty(), from either io.h when on Windows or unistd.h when on POSIX. If no data is being piped through STDIN, and no arguments were passed, will print usage and return 0; just like calling it with -h would.

@luizberti
Copy link
Contributor Author

This addressed issues brought up by #177 and #178

/cc @saper @xzyfer

@xzyfer
Copy link
Contributor

xzyfer commented Jul 28, 2016

Code formatting aside I'm ok with this. If specific implementation issues arise we can deal with them on a case by case basis, but IMHO this is an ideal approach from the user's perspective.

@xzyfer
Copy link
Contributor

xzyfer commented Jul 28, 2016

@am11 any thoughts on the Windows side of things?

@@ -213,6 +219,8 @@ int main(int argc, char** argv) {
#ifdef _WIN32
get_argv_utf8(&argc, &argv);
#endif
if ((argc == 1) && isatty(fileno(stdin))) {print_usage(argv[0]); return 0;}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: print_usage and return statements in their own lines.

@am11
Copy link
Contributor

am11 commented Jul 29, 2016

One nit, otherwise LGTM 👍

@luizberti
Copy link
Contributor Author

@am11 Sorry about the formatting, too used to having quick checks in a single line. Will fix.

@xzyfer As for Windows concerns: It was already including io.h on Windows builds, which is what contains isatty and fileno but with leading underscores. Hence those 2 defines.
As for POSIX, I just added an else to the same WIN32 ifdef that includes unistd.h, which is where POSIX keeps isatty and fileno.

It's not a big change and it should work as expected, Microsoft documentation did not mention any behaviour differences from the POSIX version, and the io.h was made exactly for providing an IO interface that is compliant with POSIX but works on Windows.

@xzyfer xzyfer merged commit 077af53 into sass:master Jul 30, 2016
@xzyfer
Copy link
Contributor

xzyfer commented Jul 30, 2016

Thanks heaps @LuizFB! Thanks again @am11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants