Skip to content

Commit

Permalink
feat(reporters): Look for color-reporter
Browse files Browse the repository at this point in the history 
When loading a reporter, look for a color-version of this.
  • Loading branch information
@budde377
budde377 committed Feb 22, 2016
1 parent 6303566 commit fd9262d
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion lib/reporter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) {
original.line, original.column)
} catch (e) {
log.warn('SourceMap position not found for trace: %s', msg)
// Fall back to non-source-mapped formatting.
// Fall back to non-source-mapped formatting.
}
}

Expand Down Expand Up @@ -103,6 +103,7 @@ var createReporters = function (names, config, emitter, injector) {
}

try {
log.debug('Trying to load reporter: %s', name)
reporters.push(injector.createChild([locals], ['reporter:' + name]).get('reporter:' + name))
} catch (e) {
if (e.message.indexOf('No provider for "reporter:' + name + '"') !== -1) {
Expand All @@ -111,6 +112,17 @@ var createReporters = function (names, config, emitter, injector) {
} else {
log.warn('Can not load "%s"!\n ' + e.stack, name)
}
return
}
var color_name = name + '_color'
if (names.indexOf(color_name) !== -1) {

This comment has been minimized.

Sign in to view

Copy link
@UnsungHero97

UnsungHero97 Oct 21, 2016

@budde377 should this be === -1? I'm seeing this in the logs:

21 10 2016 16:23:05.341:DEBUG [reporter]: Trying to load reporter: spec
21 10 2016 16:23:05.342:DEBUG [reporter]: Trying to load color-version of reporter: spec (spec_color)
21 10 2016 16:23:05.343:DEBUG [reporter]: Couldn't load color-version.
21 10 2016 16:23:05.343:DEBUG [reporter]: Trying to load reporter: jasmine-diff
21 10 2016 16:23:05.343:DEBUG [reporter]: Trying to load color-version of reporter: jasmine-diff (jasmine-diff_color)
21 10 2016 16:23:05.344:DEBUG [reporter]: Couldn't load color-version.

This comment has been minimized.

Sign in to view

Copy link
@budde377

budde377 Oct 21, 2016

Author Member

That check should ensure that we don't try and add a reporter already present. If it was already existing, this condition would be true and cause the return. I believe this is the right thing to do and it perfectly fits your provided log.

This comment has been minimized.

Sign in to view

Copy link
@UnsungHero97

UnsungHero97 Oct 22, 2016

so Couldn't load color-version is not an error? in that case, all is good, though the log message is a bit misleading.

what is the purpose of the "color-version" of a reporter?

This comment has been minimized.

Sign in to view

Copy link
@budde377

budde377 Oct 22, 2016

Author Member

It isn't an error, unless you have some reporter called spec_color or jasmine-diff_color. I'm all for clarity, but I thought that the context made it clear what was happening (e.g. trying to load.... Couldn't load....).

Anyways, having a color version of a reporter enables local color settings when e.g. starting a run from a karma client.
It's ultimately there for making the --color option work.

This comment has been minimized.

Sign in to view

Copy link
@UnsungHero97

UnsungHero97 Oct 22, 2016

got it. thanks!
return
}
try {
log.debug('Trying to load color-version of reporter: %s (%s)', name, color_name)
reporters.push(injector.createChild([locals], ['reporter:' + name + '_color']).get('reporter:' + name))
} catch (e) {
log.debug('Couldn\'t load color-version.')
}
})

Expand Down

0 comments on commit fd9262d

Please sign in to comment.