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

Can't suppress stdout for echo #899

Closed
wyardley opened this issue Nov 7, 2018 · 4 comments
Closed

Can't suppress stdout for echo #899

wyardley opened this issue Nov 7, 2018 · 4 comments

Comments

@wyardley
Copy link
Contributor

wyardley commented Nov 7, 2018

Node version (or tell us if you're using electron or some other framework):

v8.11.3

ShellJS version (the most recent version/Github branch you see the bug on):

0.8.2

Operating system:

Mac OS X

Description of the bug:

There doesn't seem to be a way to suppress stdout from echo() even when redirecting to another command / pipe.
Based on the code, it seems like no matter what, the call to sys.stdout.write() will happen.

shelljs/src/echo.js

Lines 58 to 60 in 4bd22e7

process.stdout.write(output);
return output;

Example ShellJS command to reproduce the error:

(given a file testfile without the string asdfasdf at the beginning of a line):

const shell = require('shelljs');

if (!shell.grep(/^asdfasdf/, 'testfile').stdout.trim()) {
  shell.echo({silent: true}, 'asdfasdf').toEnd('testfile');
}

In this case, whether {silent: true} (which it seems like it's supposed to be parsed, tho not do anything, based on

shelljs/src/echo.js

Lines 38 to 40 in 4bd22e7

}, {
silent: true,
});
) is there or not, the output is echoed and appended to testfile. Also, in this case, {silent: true} gets added to the file as well.

const shell = require('shelljs');

if (!shell.grep(/^asdfasdf/, 'testfile').stdout.trim()) {
  shell.config.silent = true;
  shell.echo('asdfasdf').toEnd('testfile');
}

The above code also doesn't suppress stdout.

@nfischer
Copy link
Member

nfischer commented Nov 7, 2018

Thanks for the report! Glad to hear there's more interest in this feature.

It's currently not feasible to suppress stdout with .to(), .toEnd(), etc. (technical limitations).

We're tracking an alternate redirection syntax in #800. This would remove the technical limitation, at which point I would agree we should mute output upon redirection.

I'm not sure if we'll ever be able to support silencing in pipes, unless we radically change the syntax.

@nfischer
Copy link
Member

nfischer commented Nov 7, 2018

Closing this for now, since I think we can track this in #800.

@nfischer nfischer closed this as completed Nov 7, 2018
@wyardley
Copy link
Contributor Author

wyardley commented Nov 7, 2018

@nfischer is it also not possible to silence echo() itself? Looking at the lines I mentioned above, it seems like the intent would be for echo to at least not barf on that argument?

I would have thought that just suppressing the call to process.stdout.write(output); in the case where echo() is called with {silent: true } would "fix" this?

@nfischer
Copy link
Member

nfischer commented Nov 7, 2018

which it seems like it's supposed to be parsed, tho not do anything, based on

This isn't a param to be parsed, it's an argument to our arg-parsing function (see errorOptions) which is then forwarded to common.error(). It means "if you get an error, don't print anything to the console".

in the case where echo() is called with {silent: true } would "fix" this?

echo({ silent: true }) would probably stringify the Object, which probably explains the "barf" you're seeing in the file. This certainly won't configure echo()'s behavior. I wouldn't generally recommend passing an Object to echo() like that.

So, it's WAI that echo() is not silent.


There's an argument to be made for echo() to support shell.config.silent, this just isn't the current behavior. I'd be open to changing it as a breaking change.

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

2 participants