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.

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

Added new formatter #188

Merged
merged 1 commit into from
May 9, 2017
Merged

Added new formatter #188

merged 1 commit into from
May 9, 2017

Conversation

sarvaje
Copy link
Contributor

@sarvaje sarvaje commented May 9, 2017

Fixed how formatter is used in cli.ts
Fix #187

@sarvaje
Copy link
Contributor Author

sarvaje commented May 9, 2017

This is the result: (any comment about the colors are welcome)

image

@molant
Copy link
Member

molant commented May 9, 2017

How does it look with errors?
Can you fix the conflicts too?

@alrra
Copy link
Contributor

alrra commented May 9, 2017

Can you fix the conflicts too?

@sarvaje Sorry about that :(

@alrra
Copy link
Contributor

alrra commented May 9, 2017

How does it look with errors?

@molant

@molant
Copy link
Member

molant commented May 9, 2017

Lets change where the number of error and warnings appear to the end of each resource and add the emoji stylish uses.
Everything else's looks fine to me. @alrra @ststimac @qzhou1607?

@sarvaje
Copy link
Contributor Author

sarvaje commented May 9, 2017

Can you fix the conflicts too?
@sarvaje Sorry about that :(

No problem! :)

@sarvaje sarvaje force-pushed the new-formatter branch 2 times, most recently from a84b43f to a6d668c Compare May 9, 2017 19:04
@sarvaje
Copy link
Contributor Author

sarvaje commented May 9, 2017

@molant @alrra I've changed the summary to each position, added tests and some other improvements hehehe
image

/**
* @fileoverview The basic formatter, it just a table format with diferent colors
* for errors and warnings.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't use that, I have used as reference this one https://github.com/eslint/eslint/blob/master/lib/formatters/stylish.js

t.is(t.context.logger.log.callCount, 0);
});

test(`Stylish formatter print a table and a summary for each resource`, (t) => {
Copy link
Member

Choose a reason for hiding this comment

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

Stylish formatter prints a table and a summary for each resource

const sortedMessages = _.sortBy(msgs, ['line', 'column']);
const tableData = [];

logger.log(chalk.cyan(`${resource}: ${msgs.length} issues`));
Copy link
Member

Choose a reason for hiding this comment

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

We don't need : ${msgs.length} issues. We are doing the summary after all the issues for that resource.

@sarvaje
Copy link
Contributor Author

sarvaje commented May 9, 2017

@molant done!

* Fix how `formatter` is used in `cli.ts`
* Fix #187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants