Experiment: Use JSON format and provided endLine and endColumn for highlights #196
Conversation
All esoteric syntaxes should be configured by the user.
Note that the 'crash_regex' also catches config errors.
The message eslint yields changed, so it didn't match our pattern anymore.
|
Enables something like this; multi-column errors
But also: simple indentation errors
|
|
Old look is this:
The 'if' gets marked instead of the block for the constant condition (!) error (selecting 'true' would be the minimum correct behavior here). On the last line, the first char gets selected for the indentation error. |
|
If we do this it increases the need for de-highlighting during editing. Before we start having region highlighting everywhere we need to have a design discussion about this because we’re not on the same page. |
|
Okay, but there is correct and broken. The old behavior just highlights the wrong things. Highlighting the 'if' is wrong, you could highlight 'true' of course. And the indentation is not the first space, but 5 spaces. I actually hoped that eslint would come in the org to fix some stuff here. 'De-highlighting during edit' and 'reducing noise during problem-solving' are actually my own topics. I'm the only one writing about these. EDIT: E.g. SublimeLinter/SublimeLinter#244 (comment) |
|
FWIW that the same code in VSCode. And I just think my thing looks better.
And BTW they don't have any de-emphasizing during edit. |
|
Yeah VS Code is shit, your stuff is way better. I think we agree on the noise thing. I may not write about it much but I think it’s so important I find it hard to trust anyone else with it. I should trust you more. |
|
This should be a v2 rc release, I think. It is likely that I did not handle every possible json response. (Some errors may have missing keys, or whatever.) |
|
The tooltip and status bar message only show at the start of the region. I'd expect to be able to hover over the entire region to get details about the problem.
|
|
This is not an issue here, but in core SL. |
|
Ok, awesome. Then this is really LGTM for me. |
|
And, do we merge into master (bc we never update this for SL3 anyway?) and then release a 'rc' version? We should still wait for #1007 though, so users can actually change the 'selector'. (I removed all the special 'syntax'es here!) |
|
I think the best approach would be to not release an rc but release it directly with a version 3/4 switch in linter.py. That's the easiest on end users. With flake8 we broke compatibility before we had that version to switch on, but on all other linters I prefer to use the switch over pre-releases. |
|
@kaste I guess we can merge and release this now. |
|
I'm running this PR since then. (So it will not break before we merge.) |
This is on top of #195