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

Remove TTY detection #201

Closed
wants to merge 1 commit into from
Closed

Remove TTY detection #201

wants to merge 1 commit into from

Conversation

johnsonjh
Copy link

@johnsonjh johnsonjh commented Nov 16, 2020

  • More reliable TTY detection

@johnsonjh
Copy link
Author

johnsonjh commented Nov 17, 2020

The build failure is a Travis false positive, and fixed in #222

@cjdelisle
Copy link
Member

I'd rather just delete the logic here, who cares if you log to a file when doing a --create

@johnsonjh
Copy link
Author

f978c47 apparently you cared! Haha.

I'd be fine with getting rid of it, I don't think anything we have relies on the behavior... the intention was probably to avoid inadvertently writing the seed out to disk?

@johnsonjh
Copy link
Author

@cjdelisle Pushed a replacement that just deletes the logic, if you prefer that.

@johnsonjh johnsonjh changed the title Use go-isatty for more reliable tty detection Remove TTY detection Nov 20, 2020
@johnsonjh
Copy link
Author

@johnsonjh
Copy link
Author

johnsonjh commented Dec 17, 2020

@cjd per private discussion, not using https://github.com/mattn/go-isatty - we'll just remove this check for now - https://github.com/mattn/go-isatty is a great solution, however, if you need this sort of check to be reliable.

On Windows, you have the "legacy terminal" (ala CMD or NTVDM), Windows Terminal, Cygwin Cygpty, MinGW Term, Git Bash Term, mintty, MobaXterm (Cygpty-compatible, mostly), PowerShell (launched 'alone') (vs PowerShell 7 which behaves different from the others in some ways), and PowerShell ISE (which is also different), etc... it's stupidworld over in Windows-land! I'm super-aware of just how bad it is, now that I have Windows on the test VM and setup for CI/CD.

I assume they never change or remove the old interfaces to not break older products - but it means that sometimes you need do "winpty pktwallet" and maybe Get-Content to get the result you want depending on what sort of terminal you're in and what sort of redirection or logging you want.

Example: Using PowerShell, a simple SOMEPROG < SOMEFILE that works in the legacy terminal (CMD) is NOT going to work if SOMEPROG is a "legacy" console program, you'll need to do Get-Content SOMEFILE | SOMEPROG instead. Now, if you are already in a Cygwin/MSYS-enabled console (like mintty), then running a PowerShell there, you need to do Get-Content SOMEFILE | winpty SOMEPROG. And it gets worse and worse and totally depends on the combination of the programs and your current terminal.

Happy to avoid the whole situation.

@johnsonjh
Copy link
Author

  • NOTE: Applied to WIP rebase branch successfully.

@johnsonjh johnsonjh closed this Dec 28, 2020
@johnsonjh johnsonjh deleted the isatty branch December 28, 2020 23:52
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.

2 participants