cli: add -c / -i for non-interactive use

[pillo79]

Hello, thanks for the super nice tool and hope you accept PRs!

I have added -c and -i (as in the custom sh usage) to allow calling it non-interactively from scripts or command line.
As the tool is very nicely structured, I'm not sure I've done it TheRightWay ™️ 🙂 so let me know any issues.
Also, not sure where to look to add docs! 😇

[dottspina]

Hello, thanks for the super nice tool and hope you accept PRs!

Hi @pillo79 , thanks for your kind words and for contributing, yes PR are welcome.

I have added -c and -i (as in the custom sh usage) to allow calling it non-interactively from scripts or command line.

Non-interactive sessions is indeed something I would like DTSh to support, although my initial though was batch scripts, not command line arguments.

That being said, I've tried your PR, and I'm more than fine with these -c and -i options.

I'm not sure I've done it TheRightWay ™️ 🙂 so let me know any issues.

It was the best approach you could think off without introducing other API changes.

But the design I had in mind was rather to rely on implementations of the DTShVT.readline() API that will:

• first read command lines from a batch input stream, signaling EOF when exhausted
• then, if an interactive flag is set, continue reading from the VT's standard input until EOF, otherwise signal EOF immediately

I've pushed the required refactoring and API changes in dtsh.session.DTShSession:

• add an interactive flag argument to run() and close(), execute hooks depending on whether this flag is set (e.g. skip preamble and pre-exit hooks if non-interactive)
• in run(), the session just calls DTShVT.readline() until EOF, regardless of the interactive flag and of where the command lines come from

Then implemented the batch scripts use case:

• dtsh.io.DTShInputFile: an input stream to read command lines from
• dtsh.rich.io.DTShBatchRichVT: VT with batch support (scripts or command line arguments)
• dtsh.rich.session.DTShRichSession: add batch support with a create_batch() variant of DTShSession.create()

I tested the above new API with a simple ad-hoc Python script, and it seems to work fine.

To handle the command line arguments (the -c option), would you mind:

• implementing some dtsh.io.DTShInputCmds where readline() will dequeue DTSh command lines from the dtsh command arguments (the CMD), and signal EOF when the list is empty (hint: have a look at DTShInputFile)
• using this input stream in DTShRichSession.create_batch() when the batch argument is a list (where it raises NotImplementedError)
• and eventually, using DTShRichSession.create_batch() in dtsh.cli when -c is set

Would you mind also adding the scripts use case to dtsh.cli, also with create_batch(), such that the session control options look like bellow:

  -c CMD                execute CMD on start-up
  -f FILE               execute FILE on start-up
  -i, --interactive     enter interactive loop after CMDs or FILE

Note: unlike -c CMD, -f FILE expects only one value (might be nargs=1 instead of action="append").

Do not hesitate to ask for help if this is not clear.

Also, not sure where to look to add docs!

May be adding a "Batch Support" sub-section to "Usage" in the Getting started Guide (see dtsh/doc/site/src/getting-started.rst).

Thanks.

[pillo79]

Rebased and reimplemented on the most recent dtsh-next (thanks for the infrastructure rework! 👍).

[dottspina]

First, thank for the nice work.

The requested changes are really minor, and I hope you won't find them too far fetched.

They mainly come from my preference to keep DTSh strictly typed: I'm not a pythonista and would quickly get lost otherwise. It's my fault, mypy is not configured strict in setup.cfg, only pylsp-mypy is: I'll fix that.

BTW, you may have noticed that my use of type hinting is not always consistent: this is because I'm not always sure what's the best practice. I feel Python type hinting, like everything Python, is moving really fast, and what was new in 3.8 may as well be promoted to the best practice in 3.9, then deprecated in 3.11, and so on.
Maybe you'll help me RFC all of this for DTSh once Zephyr's requirements are upgraded to Python 3.9.
​
I'm really glad we'll merge this batch support. Thanks.

[dottspina]

Could you reword the doctring: DTShInputCmds does not process command lines, it's rather some pseudo input stream.

[dottspina]

@pillo79 , please, don't apply suggestions using the GitHub interface:

• this creates a new commit with the applied changes, all in one, we don't want that
• this commit is then co-authored by me, and it's your work, not mine

Please, rebase instead and re-apply your changes from there, like you've previously done.

It's my fault, I should have used the "Comments" not "Request changes" feedbak. Sorry.

Thanks.

[pillo79]

I think suggestions are a great tool! I used it temporarily just to get the diffs so I could fixup them individually 👍

[pillo79]

Rebased on latest dtsh-next to solve conflict.

[dottspina]

I think suggestions are a great tool! I used it temporarily just to get the diffs so I could fixup them individually

It's fine, just rebase once you're done: then we'll merge the PR and I'll rebase my own work onto that.

Thanks.

[dottspina]

Rebased on latest dtsh-next to solve conflict.

@pillo79 , oops, it seems you squashed the dtsh.rich.session commit into the dtsh.io commit ?

Could you please submit 4 commits as before: io, rich.session, cli, getting-started.

Thanks.

[pillo79]

it seems you squashed the dtsh.rich.session commit into the dtsh.io commit

Nope, I always modified both files in the same commit; these changes looked very related to me... 🤷‍♂️
Obviously fine with either though!

[dottspina]

Nope, I always modified both files in the same commit; these changes looked very related to me...
Obviously fine with either though!

Maybe I hadn't noticed.

The rationale for this far fetched splitting, even for changes which may look closely related, is to keep things a bit easier to backport to the RFC/PR (which shall show as this 30 separate commits series for the sake of reviewers' health).

This batch/non-interactive support is really something that I had in mind, but was not my priority when working on DTSh. Thanks to your PR, this happened faster than I thought, and I'm really happy about it.

A bit out of topic: I'm going to push the initial implementation of the cat command, would you mind playing a little with it, just so I have minimum feedback before we can release 0.2.1 ;-)

Thanks.

[pillo79]

A bit out of topic: I'm going to push the initial implementation of the cat command, would you mind playing a little with it, just so I have minimum feedback before we can release 0.2.1 ;-)

Sure will!