Automatically fallback in non-TTY environments about listr HOT 23 CLOSED

I still think it should fallback to verbose automatically if this isn't set

It will :)

Thanks for the feedback everyone! Will go for a separate option to make it clear. I will go for nonTTYRenderer for now unless someone comes up with something better :).

from listr.

My first idea was to let the renderer check if it is run in a TTY or not and then switch to another renderer if necessary. But that would be a lot of redundant code in the renderers and error-prone (if forgot to check), of course.

👉 So my favorite option would be 2. Option 3 moves too much implementation details into the hands of the user for my taste.

Let the user select a custom renderer and let then the custom renderer decide what to do (if it supports non-TTY). If it does state support for non-TTY then fall back to verbose renderer.

Alternative: You could also let the custom renderers extend a base class that cares for the non-TTY fallback, but to be honest, I am not a big fan of class-based inheritance.

from listr.

Thanks for the feedback @andywer.

Another option that actually extends option number 2 is that the nonTTY property could also return the renderer that should be used in non-TTY environments. If it is not implemented, it will fallback to verbose, otherwise it will use the one returned by the property.

const fooRenderer = require('foo-renderer');

class CustomRenderer {

    constructor(tasks, options) { }

    get nonTTY() {
        return fooRenderer;
    }

    render() { }

    end(err) { }
}

from listr.

Yeah, but then you should better make sure you cannot run into an infinite loop.

It might be easier to just let nonTTY be true and then let the renderer code do the fallback.

from listr.

I would go for 3 since it's the simplest and most extensible one. In the option 2 I see the lack of extensibility, especially when the renderer returns another renderer. I can imagine how the next request after this one is implemented will be "allow channging the fallback renderer" so you end up with both options implemented :)

IMO verbose renderer will fit 99% users. For the rest a simple option (ttyRenderer is more clear BTW) will be enough.

from listr.

I prefer 2. The custom renderer is the most knowledgeable whether it works in non-TTY or not, not the user.

from listr.

Maybe option 2 with the possibility to change the non-TTY renderer. If that fallback renderer also returns false, it will just go for the default verbose renderer.

from listr.

Thank you all for the feedback. I'm going to add option number 2 with a boolean because as @sindresorhus noted, renderers have the knowledge of being able to run in non-TTY, not users/developers.

The only thing I have to decide is how to define fallback renderers. Might go with an array.

new Listr([
    // tasks go here
], {
    renderer: custom
});

And if you need a fallback

new Listr([
    // tasks go here
], {
    renderer: [custom, verbose]
});

If you want multiple fallbacks, in case verbose returns false for non-TTY environments as well, you can.

new Listr([
    // tasks go here
], {
    renderer: [custom, verbose, unicorn]
});

It would work the same way as fonts in CSS. If none of the renderers return true, it will pick the built-in verbose one.

from listr.

Will it also fallback by default or renderers have to be updated to gain the new functionality?

from listr.

A renderer has to implement the get nonTTY. If it does not, it's the same as returning a falsy value and it will automatically fallback to the "next" one.

from listr.

If you want multiple fallbacks, in case verbose returns false for non-TTY environments as well, you can.

So when would verbose be used then? If custom is used in TTY, and both custom and verbose returns false for non-TTY.

from listr.

The example above might be not entirely clear.

const custom = require('custom-renderer');
const unicorn = require('unicorn-renderer');

new Listr([
    // tasks go here
], {
    renderer: [custom, 'verbose', unicorn]
});

In this case, if custom returns a falsy value for the nonTTY property, it will check the verbose renderer property. Because that is a builtin one, in this case it will use the verbose renderer in non-TTY environments.

Let's take another example that might be more clear.

const custom = require('custom-renderer');
const unicorn = require('unicorn-renderer');

new Listr([
    // tasks go here
], {
    renderer: [custom, unicorn]
});

In this case it will first check the custom renderer. If that returns false, it will fallback to the unicorn renderer. If that returns true, it will use that one in non-TTY environments. If that renderer happens to also returns false, we will automatically fallback to the built-in verbose renderer which we created ourselves and we know that it works in non-TTY environments.

from listr.

I still don't get it. There are only two states, nonTTY === true and nonTTY === false. What's the point of specifying more than 1 TTY renderer?

from listr.

You are right, there is no point in doing that. And probably most of the time people would have a TTY and a non-TTY renderer specified. Or if they don't care, only a custom TTY renderer. But maybe the unicorn renderer receives an update and suddenly doesn't support non-TTY mode anymore.

The thing is that I was just explaining how it would work. I don't say it's a good thing of specifying 10 renderers, actually quite the opposite because 9 of them will never be used.

It was a suggesting how things could be implemented. I'm still undecided about it. Would you be more in favour of adding a separate option for nonTTY?

new Listr([
    // tasks go here
], {
    renderer: custom,
    nonTTYRenderer: unicorn
});

from listr.

Would you be more in favour of adding a separate option for nonTTY?

Yes, I think that would be clearer.

from listr.

I also think this behavior implies too much magic. So if something isn't working on my CI it's hard to say why. If there is nonTTYRenderer option, it becomes clear. I still think it should fallback to verbose automatically if this isn't set since I believe 99% of users won't care and expect things to just work.

from listr.

@okonet Just to be clear. It will if the default renderer set as renderer does not support nonTTY. If it supports non-TTY environments, it will use that one. If it doesn't, it will automatically fallback to verbose.

from listr.

@SamVerschueren awesome news! Can you publish a new version please? I'd really like this to be a part of lint-staged/lint-staged#108

from listr.

@okonet could you wait a little longer? Want to fix #34 first as well. Hope I can start it this evening.

from listr.

That would be even better although I'm all for smaller but more recent releases :)

from listr.

@okonet You're right. If I'm not able to do it this evening, I will release it instead. Thanks for the feedback!

from listr.

@okonet released in v0.9.0.

from listr.

Awesome! Thank you!!!

from listr.