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

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

Closed
bsnux opened this issue May 17, 2021 · 36 comments

Comments

@bsnux
Copy link

bsnux commented May 17, 2021

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 #80 (comment)

@bsnux
Copy link
Author

bsnux commented May 17, 2021

That is not displaying the output of the command

@bsnux
Copy link
Author

bsnux commented May 17, 2021

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
Copy link
Collaborator

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
Copy link
Author

bsnux commented May 17, 2021

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.

@google google deleted a comment from MTBirdman89a May 17, 2021
@antonmedv
Copy link
Collaborator

We can discuss this. What’s your idea?

@antonmedv
Copy link
Collaborator

How to disable all output in your case?

@bsnux
Copy link
Author

bsnux commented May 17, 2021

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
Copy link
Author

bsnux commented May 17, 2021

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

$.output = false

@antonmedv
Copy link
Collaborator

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
Copy link
Author

bsnux commented May 18, 2021

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
Copy link
Collaborator

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

@bsnux
Copy link
Author

bsnux commented May 18, 2021

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
Copy link
Collaborator

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

@adrianord
Copy link

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
Copy link
Author

bsnux commented May 18, 2021

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
Copy link
Collaborator

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
Copy link

@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
Copy link
Author

bsnux commented May 19, 2021

@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
Copy link
Collaborator

antonmedv commented May 20, 2021

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
Copy link
Author

bsnux commented May 20, 2021

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
Copy link

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
Copy link
Collaborator

What about this:

let p = $`yes`
p.stdout.pipe(process.stdout)
await p

@adrianord
Copy link

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
Copy link
Collaborator

I’m developing it)

@bsnux
Copy link
Author

bsnux commented May 20, 2021

@adrianord Yeah, that is my idea (#91 (comment)) and my proposed path here :)

@antonmedv
Copy link
Collaborator

Got something even more interesting:

await $`npm init`.pipe(process.stdout)

@antonmedv
Copy link
Collaborator

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

@adrianord
Copy link

adrianord commented May 20, 2021

@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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link

catpea commented May 21, 2021

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
Copy link
Collaborator

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

@bsnux
Copy link
Author

bsnux commented May 24, 2021

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

@bradparks
Copy link

bradparks commented Jan 14, 2023

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

#91 (comment)

$.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
Copy link

cben commented Dec 14, 2023

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.

  • Rarely I might want to censor a specific command even in verbose mode e.g. if it contains a secret.

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

  • Well for some commands with short output, I might be OK with logging their output too for debugging. So a third "log output if debugging" might be useful.

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 ✅
if debug always if ($.verbose) { DYI log ._command + DYI log out / .pipe(stdout) } ☹️
if debug never .quiet() + if ($.verbose) { DYI log ._command } 😦
always always $.verbose=true ✅. But for single command .quiet() + DYI log ._command + DYI log out / .pipe(stdout) ❓
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) } 😦
never never --quiet / $.verbose=false / .quiet() ✅
  • So an objective problem is ProcessPromise._command is a private undocumented API.
    @antonmedv if you lean towards letting users build their own logging, consider making an official API to get final command (as array and/or as string with shell quoting).

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

  • separate global $.knob1, $.knob2 for logging commands / outputs. (naming TBD)
    It's fine for --quiet to keep controlling both knobs.
  • per-command methods to suppress just that command / just that output. Otherwise for combinations like "if debug, never" require .quiet() suppressing both + conditional re-implementing the one I didn't want to suppress (:laugh:).

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

@cben
Copy link

cben commented Dec 14, 2023

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants