Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Stylish Formatter: output all file names #1557

Merged
merged 2 commits into from
Sep 19, 2016
Merged

Stylish Formatter: output all file names #1557

merged 2 commits into from
Sep 19, 2016

Conversation

nomaed
Copy link
Contributor

@nomaed nomaed commented Sep 18, 2016

Currently the stylish formatter prints only the first fileName, and then a list of lint errors from all files without specifying to which file they belong.
After this fix, each time a new file is reached, a new-line will be added, and then the name of the file, so each group of lint failures will be group with their fileName.

Currently the stylish formatter *only* prints the first fileName, and
then a list of lint errors from all files without specifying to which
file they belong.
After this fix, each time a new file is reached, a new line will be
added, and then the name of the file, so each group of lint failures
will be group with their fileName.
@@ -26,15 +26,24 @@ export class Formatter extends AbstractFormatter {
return "\n";
}

const fileName = failures[0].getFileName();
const outputLines: Array<string> = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

please format the type def as string[]

const fileName = failure.getFileName();
if (currentFile !== fileName) {
if (currentFile) {
outputLines.push("");
Copy link
Contributor

Choose a reason for hiding this comment

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

does this just happen once, at the beginning of the lint output? if so, I think this can be refactored out of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It happens every time a new file is reached.
The idea is that whenever we start a new file, we add an empty line before it to separate the output from the previous list of lint errors.
I'll refactor it out of the loop by adding the first file-name before it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adidahiya Actually, if I move the first file out of the loop, I still need to add a test for whether we're at the first iteration or not, so it won't really make it cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

I probably should've commented on L40 -- why would currentFile be undefined multiple times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wouldn't be. It would be only for the first file, which is the case I'm trying to catch here, to avoid adding an empty line.
It's either that, or adding the line even at the first time, and then unshifting it out of the outputLines before returning it. Although then there is a need to checked whether the array is not empty and etc.

@adidahiya adidahiya merged commit 6cb1f11 into palantir:master Sep 19, 2016
@nomaed nomaed deleted the fix-stylish-filenames branch September 19, 2016 21:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants