Skip to content
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

fix: incremental reuse leading to goals in front of the text cursor being shown #4395

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

Kha
Copy link
Member

@Kha Kha commented Jun 7, 2024

As reported on Zulip.

We expected that for sound reuse of elaboration results, it is sufficient to compare the old and new syntax tree's structure and atoms including position info, but not the whitespace in between them. However, we have at least one request handler, the goal view, that inspects the whitespace after a tactic and thus could return incorrect results on reuse. For now we implement the straightforward fix of checking the whitespace as well. Alternatives like updating the whitespace stored in the reused info tree are tbd.

This has the slight disadvantage that adding whitespace at the end of a tactic will re-execute it (or the entire body, but not the header, if the body is not a tactic block), but only up to typing the first character of the next tactic or command.

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jun 7, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 7b72458392a17d1d2bf8f302eb50d24e9f98de39 --onto 287d46e1f6a7a6136c7996367a9b203d93797d00. (2024-06-07 15:39:45)

@Kha Kha added this pull request to the merge queue Jun 8, 2024
Merged via the queue into leanprover:master with commit adfd438 Jun 8, 2024
11 checks passed
Kha added a commit to Kha/lean4 that referenced this pull request Jun 10, 2024
…eing shown (leanprover#4395)

As [reported on
Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/maybe.20a.20cache.20bug.3F).

We expected that for sound reuse of elaboration results, it is
sufficient to compare the old and new syntax tree's structure and atoms
including position info, but not the whitespace in between them.
However, we have at least one request handler, the goal view, that
inspects the whitespace after a tactic and thus could return incorrect
results on reuse. For now we implement the straightforward fix of
checking the whitespace as well. Alternatives like updating the
whitespace stored in the reused info tree are tbd.

This has the slight disadvantage that adding whitespace at the end of a
tactic will re-execute it (or the entire body, but not the header, if
the body is not a tactic block), but only up to typing the first
character of the next tactic or command.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants