fix: incremental goal state requests select incomplete snapshot #6887
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug where the goal state selection would sometimes select incomplete incremental snapshots on whitespace, leading to an incorrect "no goals" response. Fixes #6594, a regression that was originally introduced in 4.11.0 by #4727.
The fundamental cause of #6594 was that the snapshot selection would always select the first snapshot with a range that contains the cursor position. For tactics, whitespace had to be included in this range. However, in the test case of #6594, this meant that the snapshot selection would also sometimes pick a snapshot before the cursor that still contains the cursor in its whitespace, but which also does not necessarily contain all the information needed to produce a correct goal state. Specifically, at the
InfoTree
-level, when the cursor is in whitespace, we distinguish competing goal states by their level of indentation. The snapshot selection did not have access to this information, so it necessarily had to do the wrong thing in some cases.This PR fixes the issue by adjusting the snapshot selection for goals to explicitly account for whitespace and indentation, and refactoring the language processor architecture to thread enough information through to the snapshot selection so that it can decide which snapshots to use without having to force too many tasks, which would destroy incrementality in goal state requests.
Specifically, this PR makes the following adjustments:
SnapshotTask
to contain both aSyntax
and aRange
. Before,SnapshotTask
s had a single range that was used both for displaying file progress information and for selecting snapshots in server requests. For most snapshots, this range did not include whitespace, though for tactics it did. Now, thereportingRange
field ofSnapshotTask
is intended exclusively for reporting file progress information, and theSyntax
is used for selecting snapshots in server requests. Importantly, theSyntax
contains the full range information of the snapshot, i.e. its regular range and its range including whitespace.SnapshotTask
to produce a reasonableSyntax
.InfoTree
goal selection does.Info
of a snapshot at the wrong location whentrace.Elab.info
was also set.This PR is based on #6329.