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.

Sign up for GitHub

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

support relative paths in linter + spans that span multiple characters/lines #369

Merged
merged 4 commits into from
Mar 31, 2016
Merged

support relative paths in linter + spans that span multiple characters/lines #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e24029d
support relative paths
oli-obk Mar 30, 2016
40b3c44
also allow a possible end of a lint-span
oli-obk Mar 30, 2016
c5a936d
adjust tests
oli-obk Mar 31, 2016
42caf24
prefer path.join over string munching
oli-obk Mar 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,10 @@ matched by the regular expression `RE`.

The following named groups can be matched from the output:
* `file` - **[required]** the file to open. May be relative `cwd` or absolute. `(?<file> RE)`.
* `line` - *[optional]* the line the error resides on. `(?<line> RE)`.
* `col` - *[optional]* the column the error resides on. `(?<col> RE)`.
* `line` - *[optional]* the line the error starts on. `(?<line> RE)`.
* `col` - *[optional]* the column the error starts on. `(?<col> RE)`.
* `line_end` - *[optional]* the line the error ends on. `(?<line_end> RE)`.
* `col_end` - *[optional]* the column the error ends on. `(?<col_end> RE)`.
* `message` - *[optional]* Catch the humanized error message. `(?<message> RE)`.

Backslashes must be escaped, thus the double `\\` everywhere.
Expand Down
5 changes: 3 additions & 2 deletions lib/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,14 @@ export default {
success = success && !this.errorMatcher.hasMatch();
}

const path = require('path');
this.linter && this.linter.setMessages(this.errorMatcher.getMatches().map(match => ({
type: 'Error',
text: match.message || 'Error from build',
filePath: match.file,
filePath: path.isAbsolute(match.file) ? match.file : path.join(cwd, match.file),
range: [
[ (match.line || 1) - 1, (match.col || 1) - 1 ],
[ (match.line || 1) - 1, (match.col || 1) - 1]
[ (match.line_end || match.line || 1) - 1, (match.col_end || match.col || 1) - 1 ]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, could you also add this to the README.

Now that I am seeing this, the whole linter integration thingie should probably have been in error-matcher.js insteaed. Not that that has anything to do with your PR :)

]
})));

Expand Down
19 changes: 10 additions & 9 deletions spec/linter-intergration-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('Linter Integration', () => {
let directory = null;
let workspaceElement = null;
let dummyPackage = null;
const join = require('path').join;

temp.track();

Expand All @@ -34,7 +35,7 @@ describe('Linter Integration', () => {
waitsForPromise(() => {
return Promise.resolve()
.then(() => atom.packages.activatePackage('build'))
.then(() => atom.packages.activatePackage(`${__dirname}/fixture/atom-build-spec-linter/`))
.then(() => atom.packages.activatePackage(join(__dirname, 'fixture', 'atom-build-spec-linter')))
.then(() => (dummyPackage = atom.packages.getActivePackage('atom-build-spec-linter').mainModule));
});
});
Expand All @@ -46,7 +47,7 @@ describe('Linter Integration', () => {
describe('when error matching and linter is activated', () => {
it('should push those errors to the linter', () => {
expect(dummyPackage.hasRegistered()).toEqual(true);
fs.writeFileSync(`${directory}/.atom-build.json`, fs.readFileSync(`${__dirname}/fixture/.atom-build.error-match-multiple.json`));
fs.writeFileSync(join(directory, '.atom-build.json'), fs.readFileSync(join(__dirname, 'fixture', '.atom-build.error-match-multiple.json')));

waitsForPromise(() => specHelpers.refreshAwaitTargets());

Expand All @@ -61,13 +62,13 @@ describe('Linter Integration', () => {
const linter = dummyPackage.getLinter();
expect(linter.messages).toEqual([
{
filePath: '.atom-build.json',
filePath: join(directory, '.atom-build.json'),
range: [ [2, 7], [2, 7] ],
text: 'Error from build',
type: 'Error'
},
{
filePath: '.atom-build.json',
filePath: join(directory, '.atom-build.json'),
range: [ [1, 4], [1, 4] ],
text: 'Error from build',
type: 'Error'
Expand All @@ -78,7 +79,7 @@ describe('Linter Integration', () => {

it('should parse `message` and include that to linter', () => {
expect(dummyPackage.hasRegistered()).toEqual(true);
fs.writeFileSync(`${directory}/.atom-build.json`, fs.readFileSync(`${__dirname}/fixture/.atom-build.error-match.message.json`));
fs.writeFileSync(join(directory, '.atom-build.json'), fs.readFileSync(join(__dirname, 'fixture', '.atom-build.error-match.message.json')));

waitsForPromise(() => specHelpers.refreshAwaitTargets());

Expand All @@ -93,7 +94,7 @@ describe('Linter Integration', () => {
const linter = dummyPackage.getLinter();
expect(linter.messages).toEqual([
{
filePath: '.atom-build.json',
filePath: join(directory, '.atom-build.json'),
range: [ [2, 7], [2, 7] ],
text: 'very bad things',
type: 'Error'
Expand All @@ -104,7 +105,7 @@ describe('Linter Integration', () => {

it('should clear linter errors when starting a new build', () => {
expect(dummyPackage.hasRegistered()).toEqual(true);
fs.writeFileSync(`${directory}/.atom-build.json`, fs.readFileSync(`${__dirname}/fixture/.atom-build.error-match.message.json`));
fs.writeFileSync(join(directory, '.atom-build.json'), fs.readFileSync(join(__dirname, 'fixture', '.atom-build.error-match.message.json')));

waitsForPromise(() => specHelpers.refreshAwaitTargets());

Expand All @@ -119,13 +120,13 @@ describe('Linter Integration', () => {
const linter = dummyPackage.getLinter();
expect(linter.messages).toEqual([
{
filePath: '.atom-build.json',
filePath: join(directory, '.atom-build.json'),
range: [ [2, 7], [2, 7] ],
text: 'very bad things',
type: 'Error'
}
]);
fs.writeFileSync(`${directory}/.atom-build.json`, JSON.stringify({
fs.writeFileSync(join(directory, '.atom-build.json'), JSON.stringify({
cmd: `${sleep(30)}`
}));
});
Expand Down