CLI should print basic usage help when invoked interactively

[DaRosenberg]

Currently if CLI is invoked interactively (i.e. stdin is connected to a terminal) the CLI will await template input. There are probably no real-world scenarios in which input will be provided this way.

It would be better if a basic usage help was printed in this case, showing the user how the CLI can be used.

If idea is supported by maintainers, I will contribute this as a PR.

Related to #583 and #584.

[harttle]

There are probably no real-world scenarios in which input will be provided this way.

Totally agree. I didn't know we can check interactive shell when it's implemented. And help message is also important without any doubt.

Thank you for volunteering to contribute code.

[DaRosenberg]

My pleasure @harttle — this way is much better than writing our own CLI wrapper.

Both this issue and the arguments #583 could be most easily handled by using commander. Would that be acceptable, or should I avoid any external dependencies? (I see that the package currently has no runtime dependencies)

[harttle]

I'm thinking about the same question. I prefer not involving runtime dependency, especially not those require native bindings.

[DaRosenberg]

Yeah I know, on the one hand it doesn't feel good to add a dependency to a package that so far has zero.

On the other hand, doing the arg parsing and command usage in a robust and user-friendly way is reinventing the wheel with a lot of unnecessary code that will need to be tested, maintained, etc... and it will never come close to the robustness of using commander.

After thinking about this for a while and looking at the JS options we will map to, I don't think it's feasible to roll our own implementation. I would say the arguments in favor of taking a dependency on commander outweigh those against. (I will look into whether there is a way to limit that dependency only to the npx context, but I don't think there is.)

The only other option I can think of is to move the CLI script to src, add commander as a devDependency and add a rollup target that bundles the CLI script together with the commander modules. I'm not sure it's really better. It removes the external package pull during npm install but instead increases the liquidjs package size. It's kind of what the whole npm ecosystem is supposed to avoid... :)

BTW not sure what you mean by "native bindings" but I don't think commander requires any, does it?

[harttle]

I don't think commander requires any, does it?

I just checked, it has zero dependency.

I would say the arguments in favor of taking a dependency on commander outweigh those against.

I trust your judgement. Let's do it, add that dependency. Keep it simple is always a good idea, and don't think about bundling it during roll-up.

[DaRosenberg]

Work is underway.

One question: for the most part it's clear which options make sense to expose through the CLI and which ones don't. But I'm not sure about this one:

https://liquidjs.com/api/interfaces/liquid_options_.liquidoptions.html#Optional-keepOutputType

The docs say:

not working for streamed rendering

What does that mean?

[harttle]

keepOutputType is for not converting tag/filter return value to string. We don't need it (always be false) for CLI because STDOUT is always a string.

[DaRosenberg]

👍 Thanks, removed it from the PR.

[github-actions]

🎉 This issue has been resolved in version 10.5.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀