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

it would be nice if editorconfig-checker could collect instances of the same issues found in adjacent lines into a single report line #359

Closed
klaernie opened this issue Aug 14, 2024 · 4 comments · Fixed by #360
Labels
feature a feature which should be implemented

Comments

@klaernie
Copy link
Member

I've just put a bunch of generated json documents into one of our repositories, but forgot to either ignore them or adjust their indentation to fit our editorconfig.

However this lead to me getting flooded with about 10k lines of reports that the indentation was incorrect.

I think a better output format would collect consecutive issues by not outputting each line by line, but rather turning consecutive lines having the same issue into a range. Although I can imagine that this naive approach would fail in cases where each line has two errors reported (they would ping-pong, interrupting each other's consecutiveness).

@mstruebing mstruebing added the feature a feature which should be implemented label Aug 14, 2024
@mstruebing
Copy link
Member

I agree that this would be much better.
The best place to implement this logic would probably be the PrintErrors function in the error package.

The ValidationErrors list should contain all information we need

func PrintErrors(errors []ValidationErrors, config config.Config) {
for _, fileErrors := range errors {
if len(fileErrors.Errors) > 0 {
relativeFilePath, err := files.GetRelativePath(fileErrors.FilePath)
if err != nil {
config.Logger.Error(err.Error())
continue
}
if config.Format == "gcc" {
// gcc: A format mimicking the error format from GCC.
for _, singleError := range fileErrors.Errors {
var lineNo = 0
if singleError.LineNumber > 0 {
lineNo = singleError.LineNumber
}
config.Logger.Error(fmt.Sprintf("%s:%d:%d: %s: %s", relativeFilePath, lineNo, 0, "error", singleError.Message))
}
} else {
// default: A human readable text format.
config.Logger.Warning(fmt.Sprintf("%s:", relativeFilePath))
for _, singleError := range fileErrors.Errors {
if singleError.LineNumber != -1 {
config.Logger.Error(fmt.Sprintf("\t%d: %s", singleError.LineNumber, singleError.Message))
} else {
config.Logger.Error(fmt.Sprintf("\t%s", singleError.Message))
}
}
}
}
}
}

// ValidationError represents one validation error
type ValidationError struct {
LineNumber int
Message error
}
// ValidationErrors represents which errors occurred in a file
type ValidationErrors struct {
FilePath string
Errors []ValidationError
}

Right now, this is a generic message, so we would probably need to make them identifiable somehow or do some magic string comparisons.

We are more than happy to take PRs or other ideas how to implement that.

@klaernie
Copy link
Member Author

I'm definitely giving it a shot - looks doable for a novice like me. Expect a PR hopefully this afternoon.

@mstruebing
Copy link
Member

If you need help or any pointers feel free to ask in here or in an (incomplete) PR! 🚀

@klaernie
Copy link
Member Author

I'll definitely need help for writing the test case and somebody telling me how idiomatic the code is. But the core logic is working to 90% already.

klaernie added a commit to klaernie/editorconfig-checker that referenced this issue Aug 15, 2024
…ecker#359)

This changes the human output format to collect multiple identical error
messages on consecutive lines into one error message that list both the
starting and ending line.

This is still missing a properly written testcase, I so far only tested
with

    make
    (cd testfiles; ../bin/ec multiple-wrong-indentations.txt)
klaernie added a commit to klaernie/editorconfig-checker that referenced this issue Aug 16, 2024
…ditorconfig-checker#359)

This changes the human output format to collect multiple identical error
messages on consecutive lines into one error message that list both the
starting and ending line.

This is still missing a properly written testcase, I so far only tested
with

    make
    (cd testfiles; ../bin/ec multiple-wrong-indentations.txt)
@theoludwig theoludwig linked a pull request Aug 25, 2024 that will close this issue
klaernie added a commit to klaernie/editorconfig-checker that referenced this issue Aug 30, 2024
…ditorconfig-checker#359)

This changes the human output format to collect multiple identical error
messages on consecutive lines into one error message that list both the
starting and ending line.

This is still missing a properly written testcase, I so far only tested
locally.
klaernie added a commit to klaernie/editorconfig-checker that referenced this issue Sep 4, 2024
…ditorconfig-checker#359)

This changes the human output format to collect multiple identical error
messages on consecutive lines into one error message that list both the
starting and ending line.

This is still missing a properly written testcase, I so far only tested
locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature which should be implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants