Make problems ranges in problems view in sync with editor squiggles

[sandy081]

Currently VS Code does not update diagnostics during edits and rely on extensions owning those diagnostics to update them.

From @pkruk2

In case extension can't simply re-generate diagnostics (for example due to side-effects or long generation time):

• right now diagnostics are not very usable, they point to wrong location
• if updated based on file edits, they could point to correct location

My argument is that in this case not updating diagnostics is worse than updating them.

Related: #102724 #90890

[ljw1004]

I think for this discussion it would help to be very precise about terms. Let's start with a crisp explanation:

1. Currently, extensions and problemMatchers generate diagnostics.
2. Extensions have complete freedom on when to update them and in response to what. problemMatchers do not have that freedom, and many of them take a long time to run or aren't intended to be run upon edits (e.g. problemMatcher for the build action "deploy new release")
3. Each diagnostic has (1) a squiggle location in the editor, (2) a range reported in the Problems pane. It's up for debate whether the two should be identical, and indeed in Visual Studio (full product) they're not.
4. Behavior of VSCode three months ago, like that of Visual Studio, was that if you had a squiggle at line 9 columns 5-10 and you insert an additional line at line 1, then the squiggle moves down to line 10 column 5-10 - in other words it stays under the same piece of text. meanwhile the range reported in the Problems page remained unchanged.
5. Behavior of VSCode today is that if you had a squiggle at line 9 columns 5-10 and you insert an additional line at line 1, then the squiggle stays as best it can at line 9 columns 5-10, but adjusted in case there isn't actually any text in that span. In other words it now underlines a different piece of text.

@pkruk2 you said (quoted by @sandy081) "not updating diagnostics is worse than updating them". I worry these words are ambiguous:

• It's not clear if this sentence refers to the squiggle location in the editor, or the range reported in the Problems page, or both.
• Some people think that "not updating a squiggle" means leaving it under the same piece of text as it was before (so it gives the appearance of not having moved). Some people think that "not updating a squiggle" means holding it as best you can to the exact same line+column.

@pkruk2 and @sandy081 I'd love if you could restate your positions in a way that avoids these ambiguities.

Here's my position:

• We have to account for problemMatchers which aren't able to be run frequently
• In all such cases, it's better for the squiggle to remain under the same word or piece of text as it was before, even in the face of edits to the file. If an inserted line earlier in the file causes the problem to now highlight a different word from what it did before, then we're displaying an untruth to the user.
• The range reported in the problems pane shouldn't change. It should be a straightforward scrape of the output.
• This will lead to squiggle-locations showing the "range after edits have applied", while the problems pane shows "range before edits had been applied". This is liveable-with, and is the least bad solution. It's also what Visual Studio does.

[pkrukp]

@ljw1004 I meant/agree with following: "it's better for the squiggle to remain under the same word or piece of text".

But I'm not sure about next two bullet points ("range reported in the problems pane shouldn't change"). I'd prefer if it was also updated (have same range as squiggle - "range after edits have applied"). The diagnostic could be marked as "dirty" somehow to signify it's not original range and might be not precise/correct.

If user adds few lines at the begining of the file, and there's a diagnostic at the end of file, why would range in Problems Pane still point to the old location? I don't see how it is better for the user.

[natefaubion]

Just as a user who is tracking this issue with a particular plugin which only updates problems on save (due to current limitations with the process it's calling out to):

• The newish behavior is very jarring. I would agree with the above statement that this behavior is not helpful in my day-to-day editing. It just becomes noise when trying to edit and fix the problems.
• I don't have a strong opinion on whether source locations update in the problems pane. To be honest, I've never noticed the divergence.
• I don't know what goes into making sure locations track within the plugin to get the previous behavior. If it's non-trivial it sounds like a really useful feature was removed, forcing all downstream plugins to re-implement this behavior if they want it. If it's a matter of adding an API call, then I'd just PR a fix and move on.

[ljw1004]

If user adds few lines at the begining of the file, and there's a diagnostic at the end of file, why would range in Problems Pane still point to the old location? I don't see how it is better for the user.

I don't think there's any way to fully implement what you describe. Here's the problem scenario:

1. Imagine if the problemMatcher discovered an error on line 9 columns 5-10. We squiggle this, and we show it in the problems pane. But the user has the file closed.
2. Now a separate command-line utility modifies the file on disk. Typical scenario is that you did "git pull" or something, and brought in a commit where someone had modified the file. Maybe they modified the file by deleting the characters on line 9 columns 7-12 (i.e. part of the squiggle range), and then they inserted a new line at line 1.
3. Now the user opens the file.

In this case there's no way for VSCode to know where the new squiggle should be. It can't very well do a "diff" and use heuristics to tell how the squiggle has moved because they'll work sometimes and fail sometimes and lead to unpredictable user experience. Also, I think it would be slow if after a pull, VSCode got the file-change notification and immediately had to do a heuristic diff on say 1000 files that have problemMatcher-scraped problems just to see if any of them had changed.

So what should we do? We have to strike a balance between being useful to the user, and having simple+predictable rules.

I think the (full product) Visual Studio is the most elegant and simple. It treats the ranges in the problemPane as sacrosanct - views them solely as a manifestation of what was scraped from the output. If you have a file open and you edit it, then it can track where the squiggles have moved. But once you close a file then it forgets them.

In specific answer to your questions, (Q1) why would the range in ProblemsPane still point to the old location? - because the user understands the simple rule that the ProblemsPane is a direct literal scrape of the tool output with no magic involved and they like this simple predictability. (Q2) how is it better for the user? - because having a simple predictable rule for what appears in the problemsPane is important, and this delivers it, and the user doesn't get confused about why sometimes it updates and sometimes it doesn't.

All that said, I do understand and sympathize with your view too @pkruk2. You think that it's good for squiggle location and problemPane location to agree, and you think they can be made to agree in most common scenarios, and the ones where they won't agree are the ones where the squiggles would no longer be highlighting the correct word anyway, and your solution would make it work fine for users even if they close and then re-open a file which they've modified. That would also be fine for me.

[jrieken]

There are two things: the model and the view. The former is driven by extensions and the latter is us showing diagnostics, e.g as squiggles, in the panel, etc. We should not temper with the model, it's the source of truth and owned by extensions. The views however can be graceful.

Behavior of VSCode three months ago, like that of Visual Studio, was that if you had a squiggle at line 9 columns 5-10 and you insert an additional line at line 1

I actually don't know if or why this changed. Adding @sandy081 @alexdima for more insights but afaik squiggles in the editor used to be sticky.

[sandy081]

@jrieken Good question

I actually don't know if or why this changed. Adding @sandy081 @alexdima for more insights but afaik squiggles in the editor used to be sticky.

Yes this is used to be sticky and I changed this while fixing the bug - #90890 because it caused inconsistencies in UI that problems view (and also goto widget I assume) is pointing to wrong location than squiggles as latter are not getting updated.

[jrieken]

I'd say that made it worst ;-) The editor moving squiggles based on its heuristics is something good IMO and other UI pieces could treat that as intermediate source of truth. The error markers that appear in the outline do this - e.g they use the IMarkerDecorationsService and getLiveMarkers

[pkrukp]

I acknowledge @ljw1004 points:

Now a separate command-line utility modifies the file on disk. Typical scenario is that you did "git pull" or something
In this case there's no way for VSCode to know where the new squiggle should be

But at the same time a very common scenario is:

• user gets diagnostics
• user clicks on first diagnostic and starts fixing it (for example adds/deletes lines)
• user clicks on another diagnostic but it points to wrong location

In my opinion updating both squiggles and diagnostics makes most sense in this case and is most user friendly.

because having a simple predictable rule for what appears in the problems Pane is important, and this delivers it, and the user doesn't get confused about why sometimes it updates and sometimes it doesn't.

Yes, I agree. I think previously squiggles were update if:

• file was interactively edited by user
• file was changed by something (not user) but the file was opened (not sure about this one though)

In the "interactive user edit case" I think user expects that diagnostic location is updated - be it squiggles, or those in Problems view.
Not sure about the second case...

[sandy081]

@jrieken makes sense to me.

• I would revert the change and make squiggles sticky
• Update markers view to use live markers to align with squiggles and outline view.

[umanwizard]

Has this been fixed? It seems to be working now for squiggles from cargo check via rust-analyzer, and I didn't change anything about my setup.

Edit: nevermind, I seem to have gotten confused.

[ericsampson]

@sandy081 just checking that this is still an outstanding work item? An external contributor could take a swing at it if it's fairly simple (reversing the commit mentioned above?)

[sandy081]

It is still outstanding and it is not just reverting the commit. The commit I mentioned was already reverted and the remaining part is following

• Update markers view to use live markers to align with squiggles and outline view.

This is not trivial and may not be a candidate for contribution.

Thanks

[vscodenpa]

We closed this issue because we don't plan to address it in the foreseeable future. If you disagree and feel that this issue is crucial: we are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding, and happy coding!

[ericsampson]

This doesn't seem like it should be auto closed?