-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
Add the column offset to the issue model #618
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is test_custom.py part of the column offset? If not, I suggest moving to a separate PR..
Yes, it's the test for the change. |
@ericwb changes updated as requested |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see the column indicator expanded to other formatters. But for now, this works for custom.
Recently, #618 introduced column offsets to the custom formatter. But other formatters should also show the column offset. Signed-off-by: Eric Brown <browne@vmware.com>
* Add the column offset to the issue model and expose via the custom formatter * Fix import order and update auto-help * Fetch property from dict but default to 0 for backward compatibility. Update the test as well * Update test_custom.py Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
Recently, PyCQA#618 introduced column offsets to the custom formatter. But other formatters should also show the column offset. Signed-off-by: Eric Brown <browne@vmware.com>
* Add the column offset to the issue model and expose via the custom formatter * Fix import order and update auto-help * Fetch property from dict but default to 0 for backward compatibility. Update the test as well * Update test_custom.py Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
Recently, PyCQA#618 introduced column offsets to the custom formatter. But other formatters should also show the column offset. Signed-off-by: Eric Brown <browne@vmware.com>
This proposed change propagates the
col_offset
of the offending node in the AST to theIssue
data model.Just as you can report line number this allows reporting of the column offset as well.
Column offset reporting is a standard feature on many linters, also I've noticed the way that bandit is used by linter integrations like VSCode could benefit having the col offset of the reported function call, import etc.
In some cases there are two issues on a single line and this would indicate which is belongs to which report.
I noticed the custom formatter didn't have any unit tests, so I added one for this change.
Example output below: