rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[rush] Add "suppressStartupBanner" flag for CLI actions

Open dmichon-msft opened this issue 4 years ago • 6 comments
trafficstars

Summary

Adds an option suppressStartupBanner that can be specified in command-line.json to skip logging of Rush's startup banner, e.g. if the command output is meant to be parsed as JSON or piped to some other tool.

Details

Changes the startup banner to be written to a string and defer printing to command line action invocation. Suppresses logging from install-run-rush.ts unless --debug or -d are passed. Fixes #2607

How it was tested

Ran various local commands and verified presence or absence of the banner.

dmichon-msft avatar Jun 29 '21 20:06 dmichon-msft

Was talking with @octogonz about this feature yesterday.

If I were to list out some requirements, I think I'd include:

  1. Users should be able to create custom parameters that suppress banners (like --json or --csv).
  2. Rush built-in commands should be able to support parameters that suppress banners (like --json or --csv).
  3. Banners are never printed in certain situations (for example, tab-completion).
  4. Banners should always be printed in certain situations (for example, -h/--help).

And then some nice-to-haves:

  1. Users should be able to create custom commands that always suppress banners.
  2. Rush built-in commands should be able to always suppress banners.

The reason I think (1) and (2) are more important than (5) and (6) is because a parameter is able to show intent -- "suppressing a banner" is a little arbitrary, whereas a parameter like --json or --csv or perhaps --text is actually specifying an API -- you're indicating what machine-readable format will be output, which requires not including extra lines of text.

If we assume that all Rush commands are by default intended for humans, then we probably shouldn't suppress banners for any built-in commands... for i in $(rush list) might work today, but not tomorrow, because we might adjust the formatting of that command to be prettier. I think it would be safer to force script authors to use a specific output flag, like --json, for scripting. (Perhaps we could add a rush list --porcelain or rush list --text to indicate a text-based, script-friendly output, that won't be adjusted in the future.)

I would make the same argument for user commands, except that forcing a user to add a parameter to show intent requires them to make a custom parameter, add associatedCommands, to it, etc., which seems needlessly pedantic. So we probably should support user-created "always suppressed" commands.

One possible suggestion for the interface could be to add a section to command-line.json:

{
    "suppressBannerOutput": {
        /**
         * If any of these parameters are specified on the command line for any command, startup banners
         * will automatically be suppressed.
         */
        "parameters": ["--json"],

        /**
         * These commands will automatically suppress the startup banner when run.
         */
        "commands": ["my-custom-command"]
    }
}

(This would allow the user to suppress startup banners for rush list in their own monorepo if they desired, even if we don't recommend it. It also allows them to extend what today is a "magic" --json flag to include their own common scriptable formats, like --csv or --xml.)

In the future, we might even add an "always": true option to this configuration section, for a user who never wants to see a Rush startup banner.

elliot-nelson avatar Oct 09 '21 22:10 elliot-nelson

a parameter like --json or --csv or perhaps --text is actually specifying an API -- you're indicating what machine-readable format will be output, which requires not including extra lines of text.

I'm not sure I like the name --text. The names --json and --csv clearly indicate a machine-readable format, whereas --text could be misinterpreted as merely "no color codes" or "no table formatting".

Also, JSON and CSV seem like special "recommended" formats:

  • JSON: generally recommended for machines -- Node.js scripts can parse it natively, and new fields can be added without breaking compatibility.
  • CSV: generally recommended for humans -- you can open this file in Excel for sorting/filtering/slicing, and you can search it using grep because item fields appear on the same line. Note that correctly escaped CSV requires a somewhat complex NPM package to read/write. (A naive approach like array.map(x => JSON.stringify(x)).join(',') will mishandle punctuation and whitespace characters.)

(Maybe YAML could also be lumped in there, if you want comments in your output. Although none of the popular NPM packages for writing YAML seem to actually support comments.)

So perhaps all the other non-recommended formats could be grouped under a single parameter like --serialize=yaml or --serialize=text? I chose the name "serialize" because it clearly suggests that another program is supposed to be able to deserialize the output, but feel free to suggest something else.

octogonz avatar Oct 11 '21 22:10 octogonz

BTW @elliot-nelson in this discussion we've been assuming that the motivation for suppressing the banner is to enable deserialization of the output. A different motivation would be if someone simply finds the banners to be too noisy. So we could imagine a --quiet switch that also suppresses the banner, but with less guarantees about the output tidiness.

And speaking of guarantees: If the tool aborts with an error, ideally --json mode should emit the error message in JSON format.

octogonz avatar Oct 11 '21 22:10 octogonz

@octogonz , @elliot-nelson , I'm pretty much at the point where I would rather only display the banner when --help (or maybe --debug) are passed, and suppress it the rest of the time. Rush adding boilerplate to custom commands is a nuisance more than a help. Similar to the version-selector code in Heft (which we should add a separate bin entry for that doesn't run the version selector).

dmichon-msft avatar Oct 11 '21 23:10 dmichon-msft

@dmichon-msft @octogonz I guess I'd hate to lose the "normal" banners all at once just because it's too complicated to hide them some of the time.

Maybe the first step is a lot simpler than either PR idea -- just a "rush --quiet list"?

That seems like it satisfies almost all the would-be script and pipeline uses of rush commands, including built in and custom. And would probably be quite easy to do (most of the work is in this PR).

elliot-nelson avatar Oct 13 '21 00:10 elliot-nelson

I don't want look too pushy, but just in case it might be useful, there is an API from nodejs for having debug output: https://nodejs.org/api/util.html#utildebuglogsection-callback and looks that the banner and other thing might use that in combination with the already existing env NODE_DEBUG.

Hope it helps.

M3kH avatar Dec 13 '21 12:12 M3kH