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

generateRange function fails for output from pylint #258

Open
kungfujohnjon opened this issue Jul 15, 2021 · 3 comments
Open

generateRange function fails for output from pylint #258

kungfujohnjon opened this issue Jul 15, 2021 · 3 comments

Comments

@kungfujohnjon
Copy link

The generateRange function has a check that compares the column number to the length of the line. In cases where the column number is greater than the line length, atom-linter throws an error and fails. For pylint (and possibly other linters), the newline is included as a column and its index is returned for syntax errors, but the line length checked by generateRange() doesn't include this character. This is causing linter-pylint to fail in these cases with an error of e.g.:

Error: Column start (6) greater than line length (5) for line 14

This could be addressed by modifying main.js lines 42-44 to the following:

  const lineText = buffer.lineForRow(lineNumber);
  const columnGiven =
    typeof column === "number" && Number.isFinite(column) && column > -1 && column <= lineText.length;

This would prevent errors of this type and set colStart to the beginning of indentation for that line.

@steelbrain
Copy link
Owner

Columns in Atom are zero-indexed, instead of 1-indexed, so <= length would be incorrect. Can you apply the -1 on ranges from pylint and see if that resolves your issue?

@kungfujohnjon
Copy link
Author

It's probably not the newline then. The other indices returned from pylint seem to be accurate, but certain syntax errors in pylint (pylint 2.x at least) consistently give an index of lineText.length+1, and according to its self tests, this seems to be the expected return value. I don't see any other documentation for what that index is supposed to mean, but regardless, it seems there should at least be a way to suppress this exception for certain linters and set the index to either the end of the line or zero.

Since generateRange is doing the work of retrieving the lineText.length itself, there's not really a way to know it a priori without repeating much of the logic from generateRange beforehand to grab the line from the buffer and modify the column index outside of the function.

@kungfujohnjon
Copy link
Author

But overall, I don't think I see the utility in ignoring the column variable if it isn't a number or the value is less than zero but throwing an error if the number is greater than lineText.length. I feel like it makes more sense that both edge cases should be treated the same way, or that in the event that colStart is greater than or equal to lineText.length, that it be set to lineText.length - 1.

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

No branches or pull requests

2 participants