google / zx

A tool for writing better scripts
https://google.github.io/zx/
Apache License 2.0
43.1k stars 1.09k forks source link

$verbose = false is not displaying command output. verbose should be just for the command #91

Closed bsnux closed 3 years ago

bsnux commented 3 years ago

Works for me.

⌘ zx ⎇ main ❯❯❯ cat foo.mjs
#!/usr/local/bin/node
import { $ } from './index.mjs'
$.verbose = false
await ($`uptime`)
⌘ zx ⎇ main ❯❯❯ node foo.mjs
⌘ zx ⎇ main ❯❯❯

Originally posted by @antonmedv in https://github.com/google/zx/issues/80#issuecomment-841612690

bsnux commented 3 years ago

That is not displaying the output of the command

bsnux commented 3 years ago

Just to clarify:

#!/usr/local/bin/node
import { $ } from 'zx'
$.verbose = false
await ($`uptime`)

Expected Output:

9:46  up 23 mins, 1 user, load averages: 2.17 2.65 5.27

Then:

#!/usr/local/bin/node
import { $ } from 'zx'
$.verbose = true
await ($`uptime`)

Expected Output:

$ uptime
9:46  up 23 mins, 1 user, load averages: 2.17 2.65 5.27
antonmedv commented 3 years ago

Expected Output:

this is not how I design the system. In non-verbose mode nothing is displayed and user decides what to show. Just console.log needed stuff.

bsnux commented 3 years ago

I see. Just wondering if you can change that to work like bash when -x flag is used. Bash always displayed the output of the command and the command itself only when x flag is present there.

antonmedv commented 3 years ago

We can discuss this. What’s your idea?

antonmedv commented 3 years ago

How to disable all output in your case?

bsnux commented 3 years ago

I'd like to disable just printing out the command itself. I still would like to get the command output itself. One use case is redirection. Example:

node script.js > output.txt

Right now, we'll get the command itself listed in output.txt. I only would like to get the standard output of the command.

This is the suggested patch to be applied:

--- index.mjs
+++ index.mjs.original
@@ -53,12 +53,12 @@
     let child = exec($.prefix + cmd, options)
     let stdout = '', stderr = '', combined = ''
     child.stdout.on('data', data => {
-      process.stdout.write(data)
+      if ($.verbose) process.stdout.write(data)
       stdout += data
       combined += data
     })
     child.stderr.on('data', data => {
-      process.stderr.write(data)
+      if ($.verbose) process.stderr.write(data)
       stderr += data
       combined += data
     })
bsnux commented 3 years ago

I think all output should not be disabled. However, if that is a requirement I'd add a new command:

$.output = false
antonmedv commented 3 years ago

Verbose is like debug. Turn on or off. If you need redirect maybe it’s better to be explicit and console.log only things you meed to? What if first command must be silenced and second redirected?

bsnux commented 3 years ago

I think "verbose" means additional information, not only command output. I agree "verbose" is like "debug". I'm thinking of as a regular CLI when all output is redirected. That's the reason I think $.verbose = true should display additional info, such as the command itself and $.output = false will omit the output of the command when $.verbose = false. So that combination will do same as right now. However, $.verbose= false and $.output = true will print out only the output of the command.

$.verbose = false
$.output = true           // it will be default, it's here just to clarify
await ($`uptime`)

Output:

9:46  up 23 mins, 1 user, load averages: 2.17 2.65 5.27

Then:

$.verbose = true          // it will be default, it's here just to clarify
$.output = true           
await ($`uptime`)

Output

$ uptime
9:46  up 23 mins, 1 user, load averages: 2.17 2.65 5.27
antonmedv commented 3 years ago

I get this. But what not $.verbose = false Console.log(
await ($uptime) )

bsnux commented 3 years ago

Main reason for not doing that is just for avoiding verbosity. If we have many commands in the same script, then the script will use much more lines. I'm thinking of zx as a shell script so it's very usual to include many commands in the same script.

antonmedv commented 3 years ago

You can combine them into one big call to $. I don’t like idea if adding another option to manipulate verbosity.

adrianord commented 3 years ago

I'm also looking for an option to turn off outputing the command that is being ran. The documentation stating that $.verbose is like turning on set -x is a bit misleading. I assumed setting it to false would be like using set +x where commands would still output to stdout but without the command itself being output to stdout.

bsnux commented 3 years ago

My idea is to use $.verbose=true as set -x. However, $.verbose=false will just print command output to stdout. The idea of using an additional option/flag is just to omit the output of the command in stdout, as you want to.

antonmedv commented 3 years ago

Again, what about just this?

let p = await $`...`
console.log(p.toString())

Or via helper like this:

function print(p) {
    console.log(p.toString())
    return p
}

print(await $`...`)

This way we don't need to introduce another API, and this is good.

adrianord commented 3 years ago

@antonmedv commands that output stdout to show progress will only show their full output at the end of execution with the work around you are proposing, this is not desirable. With using verbose they output to stdout as they are running.

bsnux commented 3 years ago

@antonmedv I think your workaround is adding unneeded complexity. I'm still thinking of zx to behave similar to bash. That's the reason I'm proposing my patch.

antonmedv commented 3 years ago

Consider next example:

#/bin/bash
date # Printed.
FOO=$(pwd) # Captured.

And same in zx:

$.verbose = false
await $`date`
let foo = await $`pwd`

In JS there is no way to tell what second command is captured and not needed to be printed. Only user can decide what should be redirected to stdout of zx script.

Or we can think of something line redirection API.

bsnux commented 3 years ago

That's my point. In your zx example below, the output of date won't be printed. However, in the bash example, it will be.

Can we see if we it's possible to capture the second one without printing it out?

adrianord commented 3 years ago

I would be fine doing something like

$.verbose = false
$.output = true
await $`date`
$.output = false
let foo = await $`pwd`

personally. I think it's unrealistic to for zx to be one to one with bash, but I'd like the option to not display the command that is being ran.

antonmedv commented 3 years ago

What about this:

let p = $`yes`
p.stdout.pipe(process.stdout)
await p
adrianord commented 3 years ago

Your example doesn't appear to work.

~
❯ cat test.mjs
let p = $`yes`
p.stdout.pipe(process.stdout)
await p
~
❯ zx test.mjs
$ yes
file:///home/aordonez/test.mjs:2
p.stdout.pipe(process.stdout)
         ^

TypeError: Cannot read property 'pipe' of undefined
    at file:///home/aordonez/test.mjs:2:10
    at ModuleJob.run (internal/modules/esm/module_job.js:152:23)
    at async Loader.import (internal/modules/esm/loader.js:166:24)
    at async file:///home/aordonez/.nvm/versions/node/v14.15.4/lib/node_modules/zx/zx.mjs:57:5
antonmedv commented 3 years ago

I’m developing it)

bsnux commented 3 years ago

@adrianord Yeah, that is my idea (https://github.com/google/zx/issues/91#issuecomment-845278061) and my proposed path here :)

antonmedv commented 3 years ago

Got something even more interesting:

await $`npm init`.pipe(process.stdout)
antonmedv commented 3 years ago

This code already in the master branch and can be tested!

adrianord commented 3 years ago

@antonmedv I guess my only gripe is what about commands that output to both stdout and stderr. Some programs print their warnings to stderr.

Would it be possible to also have a pipeErr or something?

antonmedv commented 3 years ago

@adrianord for this specific case you need to do something like this:

$.verbose = false
await $`program 2>&1`.pipe(process.stdout)

Or if you want to redirect in to stderr:

  $.verbose = false
  let p = $`program`
  p.stdout.pipe(process.stdout)
  p.stderr.pipe(process.stderr)
  await p
antonmedv commented 3 years ago

With this new pipe() method some crazy things is possible to do.

let {stdout} = await $`echo "hello"`
  .pipe($`awk '{print $1" world"}'`)
  .pipe($`tr '[a-z]' '[A-Z]'`)
assert(stdout === 'HELLO WORLD\n')
catpea commented 3 years ago

Please forgive me this is off-topic, but this is the only place I can post this.

I've been looking at proxy-www which I found on Modern Javascript: Everything you missed over the last 10 years (2020) via a post on hn wondering if it would be possible to simplify the syntax you demonstrated to something like this:

let {stdout} = await $.pipes
.echo(`"hello"`)
.awk(`'{print $1" world"}'`)
.tr(`'[a-z]' '[A-Z]'`)

or even something like this, maybe (using Korn shell if available):

let {stdout} = await $.bin.korn
.echo(`-ne "hello\n"`) // -ne is just an example arg to show that everything is still pretty much the same.
.awk(`'{print $1" world"}'`)
.program(`2>&1`)
.bork(`2>/dev/null`)
.ls('-lh', '/usr') // <-- look here it is a raw arg array for node's child_process.spawn you grab it with ...arg
.tr(`'[a-z]' '[A-Z]'`)

Again, apologies for the off-topic post. Your program is great, love the take on Knuth's Literate Programming where you allow zx examples/index.md

antonmedv commented 3 years ago

I think the pipe method is okay and should work. @bsnux what do you think?

bsnux commented 3 years ago

@antonmedv I'm OK with the pipeline method! Thanks!

bradparks commented 1 year ago

For anyone arriving here, and wanting to log only the output of a command, not the commands themselves

https://github.com/google/zx/issues/91#issuecomment-845787197

$.verbose = false
await $`program 2>&1`.pipe(process.stdout)

The only way I could get piped commands to actually be captured, and then output it, was the following, which I'm sure could be done better, but was the only thing that worked for me - note I did try using the

.pipe($`someCommand`) 

option, but no luck for me, lol! This allows me to just copy some stuff I try at the command line straight in as well, so not too bad :D

let it = (await $`cat db.template.sql | grep 'CREATE TABLE'`).stdout;
console.log(it);
cben commented 11 months ago

EDIT: can ignore most of this, skip to next comment

A different angle: personally I'm happy with the existing control of where command output goes.
Some commands I want go to user, some capture for internal parsing, some both (and for that capture + log is easy enough).

What I'm missing though is a globally controlled mode to log ALL commands, and just the commands. Exactly like set -x. I want to let my script's users to control that e.g. --quiet, but by default I don't want it to affect what goes to stdout/stderr.

In particular, IMHO it's none of user's concern whether its output was captured and re-printed vs. directly sent to stdout/stderr.

Ahem OK so I guess there are uses for different combinations 🃏... Let's see which are easy/hard now:

log command dump output how
if debug if debug default :white_check_mark:
if debug always if ($.verbose) { DYI log ._command + DYI log out / .pipe(stdout) } :frowning_face:
if debug never .quiet() + if ($.verbose) { DYI log ._command } :frowning:
always always $.verbose=true :white_check_mark:. But for single command .quiet() + DYI log ._command + DYI log out / .pipe(stdout) :question:
always if debug if (!$.verbose) { DYI log ._command }
always never --quiet / $.verbose=false / .quiet() + DYI log ._command
never always --quiet / $.verbose=false / .quiet() + DYI log out / .pipe(stdout)
never if debug .quiet() + if ($.verbose) { DYI log / .pipe(stdout) } :frowning:
never never --quiet / $.verbose=false / .quiet() :white_check_mark:

The rest are "niceties" one could DIY, but for centralized "if debug" knob to become more useful, I think would be great to get:

EDIT: zx's log function can help with many DIY entries in table above. It takes LogEntry structs, same as $.log hook receives :point_down:

cben commented 11 months ago

Oh wait, $.log does offer a way to log all commands, without output — or all kinds of other flexibility :heart_decoration:

All possible LogEntry types; here are examples of the 2 I cared about:

{ kind: 'cmd', cmd: 'git branch --show-current', verbose: true }
{
  kind: 'stdout',
  data: <Buffer 63 61 6e 64 69 64 61 74 65 2d 77 69 70 0a>,
  verbose: true
}

E.g. this was enough for me:

$.log = ({ kind, cmd, verbose }) => {
  // I want default quiet, so using my own env var instead of zx's `--quiet` flag.
  // Also checking per-LogEntry `verbose` allows silencing particular command with `.quiet()`!
  if (kind === 'cmd' && process.env.VERBOSE && verbose) {
    console.error(`+ ${cmd}`);
  }
};

Awesome.