Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: print clang-format output #186

Merged
merged 26 commits into from
Jul 17, 2018
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3bdc619
Started working using regex
lee-tammy Jul 8, 2018
ab8bc97
Working on apply patches in jsdiff
lee-tammy Jul 9, 2018
0fa2476
Able to separate string into multiple diffs and print them
lee-tammy Jul 9, 2018
314e3ca
Added comments and changed formatting -- Has special character bug
lee-tammy Jul 9, 2018
d692f24
Changed output formatting -- does not support utf8 characters
lee-tammy Jul 10, 2018
7f34080
Revert changes in a comment
lee-tammy Jul 10, 2018
bb5f697
Changed original substring function to special substring that counts …
lee-tammy Jul 10, 2018
22933d6
Added dependencies
lee-tammy Jul 10, 2018
36d0299
Made some changes
lee-tammy Jul 11, 2018
13a78ad
Split functions and added comments -- possible error when directory i…
lee-tammy Jul 11, 2018
ff94ab0
Forgot to add diff dependency
lee-tammy Jul 11, 2018
e4d1945
Added some comments and added some error checks
lee-tammy Jul 12, 2018
5c6f5f5
Updated regex -- should work on node 6 now
lee-tammy Jul 12, 2018
7844b05
Added a test for formating
lee-tammy Jul 13, 2018
b46b96e
Commit for merge
lee-tammy Jul 13, 2018
b7f7c31
Merged conflicts
lee-tammy Jul 13, 2018
850a455
Added tests and made minor changes
lee-tammy Jul 14, 2018
9b4521a
Fixed comment
lee-tammy Jul 16, 2018
8f02bce
Used lint and format
lee-tammy Jul 16, 2018
a37c01b
Changed existing tests
lee-tammy Jul 16, 2018
ddd5d81
Removed Logger import statement b/c it is not used anymore
lee-tammy Jul 16, 2018
d8575d4
Fixed tests and added one to check for error
lee-tammy Jul 17, 2018
bc47938
Fixed line over 80 chars
lee-tammy Jul 17, 2018
b57c458
forgot to run lint
lee-tammy Jul 17, 2018
69b3211
Merge branch 'master' into print-format-lines-v3
kjin Jul 17, 2018
2b80caa
Merge branch 'master' into print-format-lines-v3
kjin Jul 17, 2018
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
Prev Previous commit
Next Next commit
Added a test for formating
lee-tammy committed Jul 13, 2018
commit 7844b0575e98d4fb423fc19cf8fc1e354d4013b0
4 changes: 2 additions & 2 deletions src/format.ts
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ import * as jsdiff from 'diff';
import * as entities from 'entities';
import * as fs from 'fs';
import * as path from 'path';
import {file, fileSync} from 'tmp';

import {Options} from './cli';
import {createProgram} from './lint';
@@ -156,6 +155,7 @@ function getReplacements(fileXML: string): Replacement[] {
const replacements: Replacement[] = [];

let xmlLines = fileXML.split('\n');
// The first and last two elements in xmlLines are not needed
xmlLines = xmlLines.slice(1, xmlLines.length - 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: It wasn't immediately obvious to me that xmlLines.length - 2 is correct. If you define xmlLines as fileXML.trim().split('\n') then it will guarantee one element per non-empty line, so you could probably use xmlLines.length - 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


for (let i = 0; i < xmlLines.length; i++) {
@@ -239,7 +239,7 @@ function printDiffs(diffs: jsdiff.IUniDiff, options: Options) {
} else if (line[0] === '+') {
options.logger.log(' ' + chalk.green(line));
} else {
options.logger.log(' ' + chalk.black(line));
options.logger.log(' ' + chalk.gray(line));
}
});
options.logger.log('\n');
36 changes: 34 additions & 2 deletions test/test-format.ts
Original file line number Diff line number Diff line change
@@ -25,8 +25,11 @@ import {nop} from '../src/util';
import {withFixtures} from './fixtures';

// clang-format won't pass this code because of trailing spaces.
const BAD_CODE = 'export const foo = [ 2 ];';
const GOOD_CODE = 'export const foo = [2];';
const BAD_CODE = '//🦄\nexport const foo = [ \"2\" ];';
Copy link
Contributor

Choose a reason for hiding this comment

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

As we discussed in person, let's have the unicorn be in its own test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

const GOOD_CODE = '//🦄\nexport const foo = [\'2\'];';

const CLANG_FORMAT_MESSAGE =
'clang-format reported errors... run `gts fix` to address.';

const OPTIONS: Options = {
gtsRootDir: path.resolve(__dirname, '../..'),
@@ -210,3 +213,32 @@ test.serial('format should return error from failed spawn', async t => {
format.clangFormat.spawnClangFormat = original;
});
});

test.serial(
'format should print suggestions for fixes for ill-formatted file', t => {
return withFixtures(
{
'tsconfig.json': JSON.stringify({files: ['a.ts']}),
'a.ts': BAD_CODE
},
async () => {
let output = '';
const newLogger = Object.assign({}, OPTIONS.logger, {
log: (n: string) => {
output += n;
}
});
const options = Object.assign({}, OPTIONS, {logger: newLogger});

await format.format(options, [], false);
if (output.search(CLANG_FORMAT_MESSAGE) === -1) {
t.fail('Does not print ...run \'gts fix\'...');
} else if (
output.indexOf('+export const foo = [\'2\'];') === -1 ||
output.indexOf('-export const foo = [ \"2\" ];') === -1 ||
output.indexOf('🦄') === -1) {
t.fail('Output messages and suggested fix are incorrect');
}
t.pass();
});
});