search

purescript / spago

🍝 PureScript package manager and build tool
BSD 3-Clause "New" or "Revised" License
793 stars 132 forks source link

Make printing `purs` commands optional #154

Closed starper closed 5 years ago

starper commented 5 years ago

I think it would be better to add something like --debug flag for it.

Because some messages can be really big. Like, for example, this one:

Running command: `purs compile  ".spago/aff/v5.1.0/src/**/*.purs" 
".spago/ansi/v5.0.0/src/**/*.purs" ".spago/arraybuffer-types/v2.0.0/src/**/*.purs" 
".spago/arrays/v5.2.0/src/**/*.purs" ".spago/avar/v3.0.0/src/**/*.purs" 
".spago/bifunctors/v4.0.0/src/**/*.purs" ".spago/catenable-lists/v5.0.1/src/**/*.purs" 
".spago/console/v4.2.0/src/**/*.purs" ".spago/const/v4.1.0/src/**/*.purs" 
".spago/contravariant/v4.0.0/src/**/*.purs" ".spago/control/v4.1.0/src/**/*.purs" 
".spago/datetime/v4.1.1/src/**/*.purs" ".spago/debug/v4.0.0/src/**/*.purs" 
".spago/distributive/v4.0.0/src/**/*.purs" ".spago/effect/v2.0.1/src/**/*.purs" 
".spago/either/v4.1.1/src/**/*.purs" ".spago/enums/v4.0.1/src/**/*.purs" 
".spago/exceptions/v4.0.0/src/**/*.purs" ".spago/exists/v4.0.0/src/**/*.purs" 
".spago/foldable-traversable/v4.1.1/src/**/*.purs" ".spago/foreign/v5.0.0/src/**/*.purs" 
".spago/foreign-generic/v8.1.0/src/**/*.purs" ".spago/foreign-object/v2.0.1/src/**/*.purs" 
".spago/free/v5.1.0/src/**/*.purs" ".spago/functions/v4.0.0/src/**/*.purs" 
".spago/functors/v3.1.1/src/**/*.purs" ".spago/gen/v2.1.0/src/**/*.purs" 
".spago/generics-rep/v6.1.1/src/**/*.purs" ".spago/globals/v4.0.0/src/**/*.purs" 
".spago/identity/v4.1.0/src/**/*.purs" ".spago/integers/v4.0.0/src/**/*.purs" 
".spago/invariant/v4.1.0/src/**/*.purs" ".spago/js-date/v6.0.0/src/**/*.purs" 
".spago/lazy/v4.0.0/src/**/*.purs" ".spago/lcg/v2.0.0/src/**/*.purs" 
".spago/lists/v5.4.0/src/**/*.purs" ".spago/math/v2.1.1/src/**/*.purs" 
".spago/maybe/v4.0.1/src/**/*.purs" ".spago/mmorph/v5.1.0/src/**/*.purs" 
".spago/newtype/v3.0.0/src/**/*.purs" ".spago/node-buffer/v5.0.0/src/**/*.purs" 
".spago/node-child-process/v6.0.0/src/**/*.purs" ".spago/node-fs/v5.0.0/src/**/*.purs" 
".spago/node-path/v3.0.0/src/**/*.purs" ".spago/node-streams/v4.0.0/src/**/*.purs" 
".spago/nonempty/v5.0.0/src/**/*.purs" ".spago/now/v4.0.0/src/**/*.purs" 
".spago/nullable/v4.1.1/src/**/*.purs" ".spago/ordered-collections/v1.6.0/src/**/*.purs" 
".spago/orders/v4.0.0/src/**/*.purs" ".spago/parallel/v4.0.0/src/**/*.purs" 
".spago/partial/v2.0.0/src/**/*.purs" ".spago/pipes/v6.0.0/src/**/*.purs" 
".spago/posix-types/v4.0.0/src/**/*.purs" ".spago/prelude/v4.1.0/src/**/*.purs" 
".spago/proxy/v3.0.0/src/**/*.purs" ".spago/psci-support/v4.0.0/src/**/*.purs" 
".spago/quickcheck/v6.1.0/src/**/*.purs" ".spago/random/v4.0.0/src/**/*.purs" 
".spago/record/v2.0.0/src/**/*.purs" ".spago/refs/v4.1.0/src/**/*.purs" 
".spago/spec/v3.1.0/src/**/*.purs" ".spago/st/v4.0.0/src/**/*.purs" 
".spago/strings/v4.0.1/src/**/*.purs" ".spago/tailrec/v4.0.0/src/**/*.purs" 
".spago/transformers/v4.2.0/src/**/*.purs" ".spago/tuples/v5.1.0/src/**/*.purs" 
".spago/type-equality/v3.0.0/src/**/*.purs" ".spago/typelevel-prelude/v4.0.0/src/**/*.purs" 
".spago/unfoldable/v4.0.0/src/**/*.purs" ".spago/unsafe-coerce/v4.0.0/src/**/*.purs" 
"src/**/*.purs" "test/**/*.purs"`

And, with the latest spago, this message is now printed every time build command is called.

JordanMartinez commented 5 years ago

Do you think that --debug is a bit too broad of a name? What about something more specific, such as --no-purs-print? Also, if you are using something that runs build internally (i.e. bundle), you can use the -s/--no-build flag for the time being.

starper commented 5 years ago

@JordanMartinez I used --debug just as an example. I don't have any strong opinion about what name this flag should have. But, as far as I understand, printing purs commands was introduced mostly for debugging purposes (that's why I chose --debug), so I'm not sure that it should be a default behaviour. I think message should be printed with --purs-print, not hidden with --no-purs-print.

f-f commented 5 years ago

@starper good points. I think the --debug flag fits here (because this was introduced for debugging as you said)

JordanMartinez commented 5 years ago

I think message should be printed with --purs-print, not hidden with --no-purs-print.

Yeah, I agree. I later realized that --debug provided an "opt-in" approach whereas mine was an "opt-out" approach.

@f-f I suggested something other than --debug because I wondered whether there would be other debug-like flags you might want to add later. Using just --debug means all of these flags would be enabled whereas using a single flag for each would control more aspects of things and provide simpler documentation.

For example, if --debug now only includes the --purs-print idea and later adds some other debug flag, it'll need to explain that the single flag does multiple things in the spago --help output. If there are multiple single-aspect debug flags, then each would have its own explanation in the above output.

JamieBallingall commented 5 years ago

If I was guessing what flag I needed for more explicit messages on a Unix command (recognizing that Spago is cross platform) by first guess would be --verbose. I wouldn't be surprised if --verbose took some additional level so that --verbose 2 printed more information than --verbose 1

At the risk if bikesheding all the commands, I seem to recall an idea that Unix commands are supposed to be silent on success by default. Although I don't seem to be able to locate a source for that idea and it seems to have fallen out of fashion in recent years.

f-f commented 5 years ago

@JordanMartinez

@f-f I suggested something other than --debug because I wondered whether there would be other debug-like flags you might want to add later. Using just --debug means all of these flags would be enabled whereas using a single flag for each would control more aspects of things and provide simpler documentation.

"Debugging" is not a core part of the UX (i.e. daily usage), so I'd like to minimize the exposed surface as much as possible (because most of the times if you do spago -h you want to see the actually useful features instead of reading through a list of debug flags). Moreover, it's usually an "all or nothing" activity: when I troubleshoot a system I probably want the most output I can get. Of course there is such a thing as good signal-to-noise ratio, but to tweak it we can just add a logging level in the future, as @JamieBallingall suggested (think --debug 3 or --debug INFO)

I opened #155 with the fix for this issue. The flag is called --debug at the moment, but I'm open to good alternatives. Current runners:

Once we agree on a name I can cut a 0.7.2 release right away

JordanMartinez commented 5 years ago

@f-f Makes sense!

Also, I think --verbose is more fitting than --debug. @JamieBallingall has a point about that flag being used more frequently in CLI programs.

starper commented 5 years ago

@f-f I'm ok with all three variants.

f-f commented 5 years ago

Renamed the flag to --verbose, I'll cut a release

f-f commented 5 years ago

@starper: https://github.com/spacchetti/spago/releases/tag/0.7.2.0

It will be available on npm as soon as CI is done building the binaries