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

A tty? function to use in scripts #911

Merged
merged 7 commits into from
May 7, 2019
Merged

Conversation

radhikalism
Copy link

@radhikalism radhikalism commented Apr 12, 2019

Because planck is useful in scripting contexts, it may be executed sometimes in a pipeline and sometimes not. It would be useful to be able to detect whether stdin/stdout is a tty or not, in order to adapt script behavior.

Example use case: I may want a script that:

  • reads input lines (via read-line, say) if there is an upstream pipe
  • or else simply prints a help message if there is no upstream pipe.

Currently in planck, there seems to be no builtin way to detect stdin being a tty, so such a script running outside a pipe, that expects to read lines from stdin, will hang until there is interactive user input at read-line.

$ cat ./echo.cljs
#!/usr/bin/env plk
(require 'planck.core)
(println (planck.core/read-line))

Fine downstream of a pipe:

$ echo foo | ./echo.cljs
foo

Waits without a pipe:

$ ./echo.cljs

... until I type a line of input:

$ ./echo.cljs
foo
foo

(I'd rather have $ ./echo.cljs print a help message, say.)

In essence, I am looking for something capable of this:

$ cat ./is-stdin-tty.cljs
#!/usr/bin/env plk
(require 'planck.core)
(println (planck.core/tty? 0))
$ echo foo | ./is-stdin-tty.cljs
false
$ ./is-stdin-tty.cljs
true

For now, one workaround is to use a bash trampoline to execute test -t 0 and pass that in via *command-line-args* to the planck script. e.g.

$ cat ./is-stdin-tty-bash.cljs
#!/usr/bin/env bash
#_(
  exec plk "$0" "$(test -t 0 && echo true || echo false)"
)
(require 'planck.core)
(println (first planck.core/*command-line-args*))

But it would be nicer to have access to isatty from <unistd.h> and gain functionality similar to Python's os.isatty or Node's TTY.isatty.

This PR is a proof-of-concept implementation for discussion. It adds a planck.core/tty? function as in the is-stdin-tty.cljs example above.

Have I missed some other way of achieving this? Is it reasonable to expect a builtin function for this?

@mfikes
Copy link
Member

mfikes commented Apr 12, 2019

@arbscht I think this looks good. Apart from the two comments above, there is also some site documentation that is manually maintained under the site tree where you could add the function in the portion covering the planck.core namespace.

You can also add an entry to the CHANGELOG.md. It looks like there is currently no entries under the "Unreleased" section, so you would add a new "Added" section and refer to this PR.

I don't see anything like this in Clojure, so I suspect there is no precedent to follow. (Perhaps in Clojure, you would delegate to some Java functionality using interop.)

Also, it doesn't seem possible to do something like (tty? *in*), right. I'm guessing if you go down that path, things might not make sense.

@mfikes
Copy link
Member

mfikes commented Apr 12, 2019

Oh, another thought: There are now a bunch of file predicates that have been recently added to planck.io. I wonder if tty? would be better placed in that namespace.

@radhikalism
Copy link
Author

Apart from the two comments above

Just quickly, are these comments on the commit? I can't see any. (If you started a review you might need to submit the whole review.)

@radhikalism
Copy link
Author

Oh, another thought: There are now a bunch of file predicates that have been recently added to planck.io. I wonder if tty? would be better placed in that namespace.

Right, I think planck.io would be best too, if you can't see a reason not to use it. I only used planck.core as a default starting point.

I don't see anything like this in Clojure, so I suspect there is no precedent to follow. (Perhaps in Clojure, you would delegate to some Java functionality using interop.)

Nothing like this is builtin in Clojure or Java as far as I know. You could use System/console for some cases but the more general isatty is typically exposed through third-party native libs in Java-land.

Also, it doesn't seem possible to do something like (tty? in), right. I'm guessing if you go down that path, things might not make sense.

Yeah, the PR doesn't support it but I do want to be able to use (tty? *in*) eventually. I think it does make sense at a conceptual level — Node for example tags its TTY streams with isTTY (undefined for others) so you can subjectively check process.stdin.isTTY without naming fd numbers.

We can probably support it at least for Readers and Writers for 0, 1 and 2 by checking object equality against *out*, *in* and *err* from tty?. I'll experiment with that, but regardless a lower-level function of fd numbers is a necessary building block, so I'd be happy to confine this PR to just that if extending support to IO objects proves non-trivial.

@mfikes
Copy link
Member

mfikes commented Apr 12, 2019

@arbscht I doubt it would help with thinking through this, but I wanted to mention that some of the underlying file open operations produce file descriptors, and those are returned as string representations (simply because they are 64-bit quantities that wouldn't fit in a JavaScript number).

So, for example:

cljs.user=> (js/PLANCK_FILE_OUTPUT_STREAM_OPEN "/tmp/planck_test.txt" true)
"140625123414496"

I don't see how this would lead to figuring out something with respect to *in* and the others, but thought I'd mention it.

I suspect you are right, in that using identical? comparing with original value that *in* is bound to, would lead to a behavior that mimics Node.

planck-cljs/src/planck/core.cljs Outdated Show resolved Hide resolved
planck-cljs/src/planck/core.cljs Outdated Show resolved Hide resolved
planck-cljs/src/planck/io.cljs Outdated Show resolved Hide resolved
@mfikes mfikes merged commit 0706e0e into planck-repl:master May 7, 2019
@mfikes mfikes added the native-sdk Public SDK requiring native support label May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
native-sdk Public SDK requiring native support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants