Skip to content

Commit

Permalink
ProblemMatcher default values improvements
Browse files Browse the repository at this point in the history
* by default regex group 0 should be used for message. If user has not
  specified the regex group - this means that message does not have one.
  Better to use whole line as default, than define regex group, which
  may not be in the regex.
* in case if line has not been defined, first line can be used to
  identify the problem.

Why are these defaults useful?

As an example I have a **py.test** output

```
tests/test_something.py .F.....................
```

`F` identifies that one of the test failed.

My problemMatcher pattern is very simple

```
"problemMatcher": {
        "owner": "python",
        "fileLocation": ["relative", "${workspaceRoot}"],
        "pattern": {
                "regexp": "([^\\s=]+) \\.*F\\.*",
                "file": 1
        }
}
```

As you can see I cannot define location. I can define message=0 on my
own, but guess that default=0 is better than 4.
  • Loading branch information
outcoldman committed Apr 17, 2016
1 parent be48a08 commit c05d681
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 14 deletions.
19 changes: 8 additions & 11 deletions src/vs/platform/markers/common/problemMatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class AbstractLineMatcher implements ILineMatcher {
return this.parseLocationInfo(data.location);
}
if (!data.line) {
return null;
return this.createLocation(1, undefined, undefined, undefined);
}
let startLine = parseInt(data.line);
let startColumn = data.column ? parseInt(data.column) : undefined;
Expand Down Expand Up @@ -903,31 +903,28 @@ export class ProblemMatcherParser extends Parser {
}
});
if (setDefaults) {
// In case if patternMatcher does not have location and message defined
// use first line as location and whole line as a message.
if (result.location) {
result = Objects.mixin(result, {
file: 1,
message: 4
message: 0
}, false);
} else {
result = Objects.mixin(result, {
file: 1,
line: 2,
column: 3,
message: 4
message: 0
}, false);
}
}
return result;
}

private validateProblemPattern(values: ProblemPattern[]): void {
let file: boolean, message: boolean, location: boolean, line: boolean;
let file: boolean;
let regexp: number = 0;
values.forEach(pattern => {
file = file || !!pattern.file;
message = message || !!pattern.message;
location = location || !!pattern.location;
line = line || !!pattern.line;
if (pattern.regexp) {
regexp++;
}
Expand All @@ -936,9 +933,9 @@ export class ProblemMatcherParser extends Parser {
this.status.state = ValidationState.Error;
this.log(NLS.localize('ProblemMatcherParser.problemPattern.missingRegExp', 'The problem pattern is missing a regular expression.'));
}
if (!(file && message && (location || line))) {
if (!file) {
this.status.state = ValidationState.Error;
this.log(NLS.localize('ProblemMatcherParser.problemPattern.missingProperty', 'The problem pattern is invalid. It must have at least a file, message and line or location match group.'));
this.log(NLS.localize('ProblemMatcherParser.problemPattern.missingProperty', 'The problem pattern is invalid. It must have at least a file match group.'));
}
}

Expand Down
71 changes: 71 additions & 0 deletions src/vs/platform/markers/test/common/problemMatcher.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
'use strict';


import assert = require('assert');

import * as Types from 'vs/base/common/types';

import Severity from 'vs/base/common/severity';
import {createLineMatcher, ProblemMatcher, ProblemPattern, ApplyToKind, FileLocationKind} from 'vs/platform/markers/common/problemMatcher';

suite('Problem Matcher', () => {
test('default location for single line matcher', () => {
let problemMatcher = <ProblemMatcher>{
owner: "external",
applyTo: ApplyToKind.allDocuments,
fileLocation: FileLocationKind.Absolute,
pattern: <ProblemPattern>{
regexp: /([a-z]+) abc/,
file: 1,
message: 0
}
};
var lineMatcher = createLineMatcher(problemMatcher);
var result = lineMatcher.handle(["filename abc"]);
assert.ok(result.match);
assert.ok(!result.continue);
assert.ok(Types.isUndefined(result.match.marker.code));
assert.equal(result.match.marker.severity, Severity.Error);
assert.equal(result.match.marker.message, "filename abc");
assert.ok(Types.isUndefined(result.match.marker.source));
assert.equal(result.match.marker.startLineNumber, 1);
assert.equal(result.match.marker.startColumn, 1);
assert.equal(result.match.marker.endLineNumber, 1);
assert.equal(result.match.marker.endColumn, Number.MAX_VALUE);
});

test('default location for multi line matcher', () => {
let problemMatcher = <ProblemMatcher>{
owner: "external",
applyTo: ApplyToKind.allDocuments,
fileLocation: FileLocationKind.Absolute,
pattern: [
<ProblemPattern>{
regexp: /file = ([a-z]+)/,
file: 1,
message: 0
},
<ProblemPattern>{
regexp: /severity = ([a-z]+)/,
severity: 1
}
]
};
var lineMatcher = createLineMatcher(problemMatcher);
var result = lineMatcher.handle(["file = filename", "severity = warning"]);
assert.ok(result.match);
assert.ok(!result.continue);
assert.ok(Types.isUndefined(result.match.marker.code));
assert.equal(result.match.marker.severity, Severity.Warning);
assert.equal(result.match.marker.message, "file = filename");
assert.ok(Types.isUndefined(result.match.marker.source));
assert.equal(result.match.marker.startLineNumber, 1);
assert.equal(result.match.marker.startColumn, 1);
assert.equal(result.match.marker.endLineNumber, 1);
assert.equal(result.match.marker.endColumn, Number.MAX_VALUE);
})
});
4 changes: 1 addition & 3 deletions src/vs/workbench/parts/tasks/test/node/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,7 @@ class PatternBuilder {
this.result = {
regexp: regExp,
file: 1,
message: 4,
line: 2,
column : 3
message: 0
}
}

Expand Down

0 comments on commit c05d681

Please sign in to comment.