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

Make --color a shakeOption #49

Closed
nh2 opened this issue Aug 29, 2013 · 9 comments
Closed

Make --color a shakeOption #49

nh2 opened this issue Aug 29, 2013 · 9 comments

Comments

@nh2
Copy link
Contributor

nh2 commented Aug 29, 2013

I would like to enable colouring by default using shakeOptions; it is not exposed there yet.

Also: Colours are great, they should be on by default! A bit of green makes your build system feel much friendlier.

@nh2
Copy link
Contributor Author

nh2 commented Aug 29, 2013

(Alternatively to making it an option, outputColor could be exposed so that I can set { shakeOutput = outputColor }.

@nh2
Copy link
Contributor Author

nh2 commented Aug 29, 2013

Also, in shakeOpts = foldl (flip ($)) baseOpts flagsShake: Granted that the flags list is short, but wouldn't foldl' be better for any case where you actually want to use the whole list?

ndmitchell added a commit that referenced this issue Aug 29, 2013
@ndmitchell
Copy link
Owner

First rule of programming, never assume command line arguments are short - every time I've done that someone has piped several Mb through the arguments (seriously, it's happened about 5 times). I've switched to foldl'.

I suspect that exposing outputColor is a better choice than adding a new member to ShakeOption. However, it doesn't currently work on Windows, which I would want to fix before exposing it. There are some docs on how that could work (see http://stackoverflow.com/a/8285944/160673) and some of that could be generalised with the Progress stuff that already uses escape sequences or console functions depending on platform.

Once it is available everywhere easily, then I can think about making it the default. In general very few programs use color by default, in particular make doesn't, and my command line flags and behaviour are make compatible by default, suggesting I shouldn't. However, there are standard conventions like picking up extra flags out of an environment variable, which I'd be quite happy to add.

In the meantime, you can easily work around this by using withArgs to customise the arguments before they get to Shake and just tack on --color. It's not beautiful, but it should work - I do this all the time in the Shake test suite.

@nh2
Copy link
Contributor Author

nh2 commented Aug 30, 2013

Once it is available everywhere easily, then I can think about making it the default. In general very few programs use color by default, in particular make doesn't

make was there before colours where invented :P

just tack on --color

Yes, using

withColors f = do args <- getArgs
                  withArgs ("--color":args) f

withColors $ shakeStuff ...

now.

@ndmitchell
Copy link
Owner

Note that npm, part of Node.js, uses colored text on Windows in the standard console quite successfully, so I can always copy what they are doing.

@Mathnerd314
Copy link
Contributor

ansi-terminal implements cross-platform colors. It also provides setTitle, so that code can go away (except for the xterm check).

@ndmitchell
Copy link
Owner

@Mathnerd314 @feuerbach - very cool, clear interface, minimal dependencies, looks like it provides what I need. However, a quick try shows it works nicely in the Windows console, but not so well in Cygwin bash console on Windows. I'll see if I can figure out why, and if I can fix that up, it would make implementing colors much easier.

@ndmitchell
Copy link
Owner

Trying ansi-terminal again with the included demo, it works perfectly in all situations (could well have been my old cygwin installation that had the issues). I should definitely switch over.

@23Skidoo
Copy link
Contributor

23Skidoo commented Mar 16, 2017

There should also be a --no-colour option, or at least some way of disabling colouring programmatically. --colour is quite useless right now, since it's enabled by default.

23Skidoo added a commit to 23Skidoo/shake that referenced this issue Mar 17, 2017
ndmitchell added a commit that referenced this issue Aug 30, 2017
ndmitchell added a commit that referenced this issue Aug 30, 2017
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

4 participants