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

Better highlights #744

Merged
merged 5 commits into from
May 31, 2024
Merged

Better highlights #744

merged 5 commits into from
May 31, 2024

Conversation

rtetley
Copy link
Collaborator

@rtetley rtetley commented Feb 21, 2024

WIP for making faster and better highlights
Adresses #743 for manual mode, need to think it through for continuous mode.
Closes #657
Closes #642

@rtetley rtetley requested a review from gares February 22, 2024 14:01
@rtetley rtetley force-pushed the better-highlights branch 3 times, most recently from 584904a to 173ad49 Compare March 28, 2024 10:17
@rtetley rtetley marked this pull request as ready for review March 28, 2024 10:17
@rtetley
Copy link
Collaborator Author

rtetley commented Mar 28, 2024

This is preliminary work. It solves the performance issues for good (making them on par with VsCoq 1 on the linked example from #657. It also creates better distinctions between executed, executing and "prepared" (i.e. the range the user has asked after an interpret to point). I am not sure how well it works with delegation.
For continuous mode highlights I will handle it in a subsequent PR.
The general mechanism here will allow us to many QOL improvements down the line.
What do you think @gares ?

@@ -291,7 +318,8 @@ let init init_vs ~opts uri ~text observe_id =
let init_vs = Vernacstate.freeze_full_state () in
let document = Document.create_document init_vs.Vernacstate.synterp text in
let execution_state, feedback = ExecutionManager.init init_vs in
let st = { uri; opts; init_vs; document; execution_state; observe_id } in
let overview = { processing = []; processed = []; prepared = []} in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: define an empty_overview since you do this twice

Comment on lines +236 to +245
if Position.compare r1.Range.start r.Range.start == 0 &&
Position.compare r1.Range.end_ r.Range.end_ == 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like Position.equal (if we derived one)

@rtetley rtetley force-pushed the better-highlights branch 2 times, most recently from fd77091 to 02fb4bf Compare May 31, 2024 12:10
rtetley added 5 commits May 31, 2024 14:15
The highlights will now be computed on the go.
This improves considerably improves performance and
will enable easily building more imformative highlights
(processed, parsed, etc...)
Some work to allow getting the decorations only until a sentence,
which allows for displaying navigation in manual mode.
We switch back the exec_overview in the document manager. Additionally we use the
Execute events to track which sentence is currently executing or processed.
When a user triggers a interpret to point, it will show up as
a prepared range until it is executing or executed.
@rtetley rtetley force-pushed the better-highlights branch from 02fb4bf to 1870fac Compare May 31, 2024 12:15
@rtetley rtetley merged commit 53bc95c into main May 31, 2024
15 checks passed
@rtetley rtetley deleted the better-highlights branch June 19, 2024 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants