chshersh / iris

🌈 Haskell CLI Framework supporting Command Line Interface Guidelines
https://hackage.haskell.org/package/iris
Mozilla Public License 2.0
173 stars 21 forks source link

fixes #58 detect non interactive terminals #83

Closed marcellourbani closed 1 year ago

marcellourbani commented 1 year ago

I'm having second thoughts on this implementation.

So I plan to check both supportsANSI and hIsTerminalDevice, and only force noninteractive if both fail

PS: should we add an option to force interactivity to support tools like expect?

chshersh commented 1 year ago

@marcellourbani The fixes look great 👍🏻 I believe, the only difficulties left are:

I'm not an expert on the first one. So I don't have a strong opinion on this. We can go with the simplest reasonable option for now and change later when it's not enough. Nobody pays me to support exotic shells. So if the standard library doesn't have an out-of-the-box cross-platform solution, what can I do 🤷🏻

I agree that the --no-input flag is responsible for multiple use cases:

  1. Disabling asking questions.
  2. Don't do interactive things like progress bars.

As for the second one, let's check only stdin. I've consulted the CLI guidelines and they recommend checking only stdin:

I believe, I confused --no-input with --no-colour a bit 🥴

So, a simple solution would be to just run hSupportsANSI on stdout and be done with it 👏🏻

marcellourbani commented 1 year ago

@chshersh

Nobody pays me to support exotic shells. So if the standard library doesn't have an out-of-the-box cross-platform solution, what can I do 🤷🏻

What about letting the user override?

data RequestedInteractiveMode
    = RInteractive
    | RNonInteractive
    | RAutoDetect

data InteractiveMode
    = Interactive
    | NonInteractive

handleInteractiveMode :: RequestedInteractiveMode -> IO InteractiveMode

or simply:

handleInteractiveMode :: Maybe RequestedInteractiveMode -> IO InteractiveMode

so by default we autodetect, but users can force either way

chshersh commented 1 year ago

@marcellourbani Overriding interactivity is something I'd like to avoid. We can't output interactive info in a non-interactive terminal so it doesn't make sense. Also, there's no appropriate CLI flag for that and since it's a global default, I'd like to avoid polluting global namespace with non-standard flags.

I'm happy to accept support of exotic shells like minTTY and Cygwin. So, if you want to work on supporting and testing them, feel free to continue working on this issue and taking as much time as you want 🤗

I was mostly implying that the changes might be unreliable without the ability to test this on CI, so the support will be potentially fragile. And if someone wants to contribute the support of these shells and verify that it works for them, I'm happy to accept this change! But I personally don't have the capacity for that. And I wouldn't ask contributors to do such huge work either 🙏🏻

marcellourbani commented 1 year ago

Thank you for the mentoring!