-
Notifications
You must be signed in to change notification settings - Fork 25
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
Line numbers in stylish and json report are off by one #22
Comments
Hey @rixx, thank you for the detailed report. No need for the reproducible demo in this case – this is plenty enough of information (and something I’ve noticed myself as well, occasionally). I vaguely remember fixing with the half-assed |
For me, Have submitted PR #47 to remove the |
I cannot reproduce this. I just installed curlylint from PyPI (0.12), and I still get the following:
|
This is strange, I get the following (also with a regular file, not just stdin), on MacOS with Python 3.9 as well as Debian 10 with Python 3.7.3. This is with my patched fork, stock curlylint shows
|
Well, the mystery thickens! I thought last time I had investigated this that compact was correct and the rest was 0-indexed as initially reported here. I’ll have another look. I think I had it almost solved but got blocked by not being able to decide where it was better to convert the numbers to be 1-indexed – at the reports level, or directly where the issues are created. |
Thicken it does! I tried the original sample input from @rixx, and it seems that the heart of the issue is actually that line numbers from some errors are 0-indexed, while others are 1-indexed:
|
Ah, looks like this mystery is in fact just coincidence. @rixx's sample used mismatched tags, whereas my sample (and the error that initially led me to comment) was an unterminated tag. Mismatched tags will report the line of the closing tag, but unterminated ones will of course make the scan move forward - in the case of my examples above, to the empty line introduced by
Therefore now fully agree with @rixx's original report, and second his suggestion that line numbers in |
Describe the bug
All line numbers in the
stylish
andjson
report format are off by one, reporting line 0 when the actual issue is on line 1. In thecompact
export format, both versions of the line number are reported.Steps to reproduce
Create a
test.html
with this content:For reference, I also tested this with indent errors by running against a file containing just
<div></div>
. The same issue occurs, but doesn't illustrate the issue with mixed content in some error messages.Run
curlylint test.html
. The output reports line number 0:Run
curlylint --format json test.html
. The output reports line number 0:Run
curlylint --format compact
. The output reports line number 1 and line number 0:Expected behavior
Consistent reporting of line numbers, preferably 1-indexed. 😉
Actual behavior
Behaviour is buggy/mixed, primarily 0-indexed, as described above. Sorry, no screenshots, but I pasted the output.
Reproducible demo
As above. Your issue template indicates that I should link a project for this, which I can do, if the one-line
test.html
is insufficient. Please let me know, in that case.Bug source
The
compact
method casts its contents tostr
(1), which comes down to theIssueLocation
class, which includes a+1
in its str() method. The other two formatters don't adjust the internal line number at all.The issue of the mixed message in the
compact
report seems to sit deeper, as it's due to the issue message, which is constructed in each rule.The text was updated successfully, but these errors were encountered: