-
-
Couldn't load subscription status.
- Fork 27.1k
Add support for non-interactive terminal #1032
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
Conversation
| Also added a commit that should fix #873 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"A numeric file descriptor", it's correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, so 1 is a magical number that means the standard output. Perhaps process.stdout.isTTY would be clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure... I've changed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was testing this branch and after making a change the terminal screen just went blank. I thought the compiler had got stuck. Why did we change this behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could only show The app is running at... after the first compile, but keep showing Compiled successfully! after each successful rebuild?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. This should be non-interactive only change. It's to prevent duplicate output in non-interactive terminal. I've just fixed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please simplify this condition by extracting some boolean variables? It's getting crowdy and hard to tell when it's true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, fixed..
| It should be ready to merge.. |
| I tested the non-interactive shell with concurrently. The output after starting the server, changing a file and waiting a few seconds looks like this: I'm not normally using a setup like this, so I'm not sure which alternative would be better, but the current behaviour doesn't seem right. |
| @fson I've changed code so it always displays "Compiled successfully!". Could we merge? |
| Thanks @sheerun, nice work! |

This should fix #869