Added -color

[lionello]

https://issues.dlang.org/show_bug.cgi?id=12273

[lionello]

PR for dlang.org: dlang/dlang.org#605

[WalterBright]

lower case w

[alexrp]

We should not print colors if !isatty (fileno (stdout)) || !isatty (fileno (stderr)). It would also be worth considering checking TERM (on Unix only): If it's set to dumb or not set at all, then colors should be disabled.

[dnadlinger]

@alexrp: I'm not sure if checking isatty in addition to having the color flag is a smart thing to do. Sure, if we want to enable colored output by default, then we need to check whether we are actually writing to a tty. But right now -color is a flag that needs to be explicitly specified anyway, and it is useful to be able to force color output in some cases.

Once such situation is with parallel build tools such as Ninja that cache the output to later print it to the console (and can't use a pseudo-tty per child process for performance/availability reasons). I found it very helpful that Clang's -fcolor-diagnostics forces colored output when using it with CMake/Ninja, and I don't see a reason why we should try to outsmart the user in DMD.

[alexrp]

@klickverbot if we don't check isatty and TERM then we're moving that burden to basically everyone using DMD and wanting colors iff DMD is being invoked from a terminal.

I agree that forcing colors can be useful, but let's have two flags then.

[dnadlinger]

@alexrp: That, or just enable them by default.

[WalterBright]

but let's have two flags then.

Please, no. Every additional switch is a failure of design, and a burden on the user.

[dnadlinger]

@WalterBright: So are you okay with just enabling colors by default, and providing an explicit on/off switch that disregards tty detection? (Edit: "by default" == "if output is going to a terminal")

[WalterBright]

@klickverbot yes that sounds better

[lionello]

I think I've addressed all the raised points. Defaults to on based on TTY/TERM, can be forced on/off with the -color or -color=[on|off] flag.

I'm a bit worried that the TTY/TERM auto-detection is a black hole. Other projects I've seen get into full-fletched terminal capability testing.

Only colors errors at the moment. What about warnings/deprecation?

[WalterBright]

Isn't it standard for a trailing '-' to mean 'off' ?

[WalterBright]

What about warnings/deprecation?

Let's see how this one plays out first.

[lionello]

@WalterBright I've checked a few other tools and grep and git use --color[=auto|always|never], whereas dmd already has an on/off switch with -boundscheck=[on|safeonly|off] so I decided to stick to on/off for -color as well. As far as I know only the "-o-" switch has a trailing '-' but it always has it (and FWIW I don't like it.)

[lionello]

@alexrp care to comment on this logic? Is this what you had in mind?

[alexrp]

Does DMD never use stdout? If it does, we should probably check isatty for that too.

[lionello]

The colors are only ever used on stderr so I only check stderr.

[alexrp]

Then it looks fine.

[ibuclaw]

@alexrp - I believe I fixed dmd diagnostics a while back to only send verbose (-vtls, etc) messages to stdout. All warnings and errors are sent to stderr.

[ibuclaw]

I'd be opined to suggest that these functions and all error/warning/deprecation functions should be in one file named diag.c or something like that.

Other than that, gdc has had this for a while. ('error' in red, 'warning' in purple, supplementary 'note' in cyan)

[lionello]

This one is good to go. Please open new bugzilla/PRs for any enhancements. I'm not sure why nobody has enabled auto-merge on this. Go go go!

[dnadlinger]

Auto-merge toggled on

[MartinNowak]

Why do we need a switch for this?
Can't the auto-detection be made reliably enough?

[dnadlinger]

@MartinNowak: See my comment above for a situation where you might want to force colored output ("proper" parallel build tools that don't randomly interleave subprocess output).

[Orvid]

Well, once again, a PR was merged that added a new file, but failed to add that file to either of the VC projects, or the xcode projects. Will make a PR to fix the MSVC projects momentarily.

[lionello]

@Orvid sorry :S Now I know

[ghost]

Well, once again, a PR was merged that added a new file, but failed to add that file to either of the VC projects, or the xcode projects.

Without auto-testing this is always going to be a problem. Especially since we're relying on this more and more often now.

[MartinNowak]

Can we automate the adding or at least testthat the list is complete without actually running Xcode or VisualStudio?

[rainers]

Can we automate the adding or at least testthat the list is complete without actually running Xcode or VisualStudio?

Testing could be done by a module in the dmd test suite that extracts the source files from the VC/XCode project files and the makefiles and compares them.
Adding a new file to a VC project manually is pretty obvious, but the XCode format is less accessible.

[Orvid]

The only thing that's really different between building and testing for MSVC is compiling DMD, for instance, I use this pair of scripts to build DMD, phobos, and druntime for 32 and 64 bit, and then also install them. (I run the scripts via git bash) The only real difference would be what is done in the build_dmd_msvc64 function. In my case I call out to a batch file which sets the environment variables I need, and then just runs msbuild. After DMD is built, you could easily run the tests in the exact same way you currently run tests for a non-MSVC build. The same can be done on OSX, except that an external script isn't needed to do the building of DMD.