Feature fix no colors to run command #1902
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This looks good but just to be sure can you add a e2e test using the mocha reporter that checks for the correct output please? |
|
@dignifiedquire Yes, I'll add it this weekend. |
|
@budde377 thanks. Do you want to join the karma maintainer team? Main rules are, keep doing great work like you did, and don't push/merge to |
|
@dignifiedquire Yes, I would very much like that :) |
|
@budde377 done, welcome to the team |
budde377
force-pushed
the
feature-fix-no-colors-to-run-command
branch
from
14ae540
to
4ca7378
Compare
|
@dignifiedquire Here you go! |
|
@budde377 this needs a rebase after I merged your other PR |
budde377
force-pushed
the
feature-fix-no-colors-to-run-command
branch
from
4ca7378
to
73b7cdf
Compare
|
@dignifiedquire, Here you go ;) |
|
Thanks. Here goes nothing |
|
Damn. There is another conflict sorry |
Add colors and log-level arguments to run argument. Refactor log-setup functions for server and init. Correct bug in server where log-level was ignored before `parseConfig` Closing karma-runner#1067
Delay the decision on color/no-color reporter to write. This is done by letting the individual adapter decide the color and providing a default color to the base adapter. Closing karma-runner#1067
Fix problems: * Not relying on Reporter.USE_COLORS property * Construct factory with right configuration
When loading a reporter, look for a color-version of this.
Add a test of the karma-mocha-reporter. Testing that it outputs correctly.
budde377
force-pushed
the
feature-fix-no-colors-to-run-command
branch
from
73b7cdf
to
76cdd60
Compare
|
No prop. This should do the trick. |
dignifiedquire
added a commit
that referenced
this pull request
Feb 22, 2016
…ommand Feature fix no colors to run command
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have not identified and fixed the problem with the 3rd-party reporters not working. The problem was that the reporters didn't update the
USE_COLORSproperty, from its default value:false. The feature relies highly on this property to be updated and when it wasn't, the reporter would be interpreted as a no-color reporter.In-order not to break backwards compatibility, a new property was introduced:
EXCLUSIVELY_USE_COLORSwithundefinedas it's default value. By using this property, the no-colours feature (in the run command) will work for default reporters, i.e. dots and process, and reporters choosing to update the property.When fixing this, it became obvious why 3rd-party reporters didn't wan't to update this property: They didn't want to implement two reporters, because this would require for their users to add two reporters to their configuration.
For instance; for the mocha reporter, used in #1894, to implement this feature they would first implement two reporters, say
reporter:mochaandreporter:mocha_color, which would yield a configuration as:where omitting one would disable reporting w/w.o colours, depending on the omitted reporter.
I fixed this in 3294a15 by attempting (silently) to load a "coloured" version of a given reporter. Thus for the mocha reporter to support this feature, all they need to do is implement and export two reporters:
reporter:mochaandreporter:mocha_color. The user-configurations would remain unchanged: I.e.Given a reporter with name
foothe coloured version must be namedfoo_color, i.e. with postfix_color.