Include both start and end line_no and column in issue

[mhanberg]

What were you trying to do?

Issues include a line_no and column field, but for editors to be able to correctly highlight the problems, they need more information.

You can see here, that only the first character of the code is underlined.

You can also see here, where the column information is not correct in general

Expected outcome

The issue includes the text document full range of the problem

Actual outcome

The issue includes only the start of the range, and that is not accurate sometimes.

[rrrene]

Hi, I notice just now that I never commented after reading the linked PR's discussion. Sorry for that.

As was discussed in the linked PR, Credo's issues have a trigger field that contains the string causing the issue (e.g. "unless" for the UnlessWithElse check).

Deriving the end_column from that should be trivial, no?

[rrrene]

Checking in 😎

[mhanberg]

Sorry, we just had a baby over here haha.

In the second screenshot the column info is just wrong, so calculating the end column using the trigger isn't going to be accurate. But assuming that is just a bug in certain checks, then yes it can be doable as long as the span shouldn't cover multiple lines.

Should the cyclomatic complexity check highlight the entire case expression or just the word case? For example.

[rrrene]

But assuming that is just a bug in certain checks

Yes. I fixed most of them three weeks back. Will be released soon.

Should the cyclomatic complexity check highlight the entire case expression or just the word case?

Here I think the trigger should always be the function's name as it is the function that is perceived as "too complex".

But CC is a really ... controversial ... check/metric. in the sense that it calls on people to change their flow to satisfy a linter.

I get those warnings for perfectly readable functions and think I would not include it again, if there every came a Credo 2.0.

[rrrene]

But assuming that is just a bug in certain checks

Yes. I fixed most of them three weeks back. Will be released soon.

@mhanberg These fixes are now live in Credo 1.7.4 🎉

[mhanberg]

Nice!

I'll go ahead and close the issue. Thanks for working through it!