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

Cancel parsing event #941

Merged
merged 25 commits into from
Dec 3, 2024
Merged

Cancel parsing event #941

merged 25 commits into from
Dec 3, 2024

Conversation

rtetley
Copy link
Collaborator

@rtetley rtetley commented Nov 6, 2024

Trying another route from #937.
This time we use the SEL api to cancel events.
Currently does not seem to be working because of problem noted in gares/sel#10

Closes #682.
Closes #914.

@rtetley
Copy link
Collaborator Author

rtetley commented Nov 13, 2024

This is a big one, still WIP. Needs:

  • Recomputing the overview when after a new parse event
  • Fixing the mechanism for getting proof views -> should be handled in the doc manager

In order to interupt a full document parse, we first need to make it part of the event loop.
We introduce parse events for each line which will allow for
better interuptibility as well as laying the ground work for
blocking on parse errors.
…parsing is over

This brings back performance to acceptable levels.
- Removed some commented out dead code
- Cleaned up ParseEvent (it doesnt need any info)
- Cleaned up the api (removed unecessary variables to init, reset, etc...)
- Removed cancel handle for ParseEvent
Now instead of launching execution directly from the lsp event,
we queue a new event that has less priority then parsing.
The operations on range lists used to compute the overview could be re-used
somewhere else
This will allow to easily recompute the overview from an
invalidated state, and makes a correspondance between an
execution state and its overview.
This avoids two consecutive execution events happening at the same time and
messing up the state.
Additionally we move the parsing cancel handle to the document where
it belongs.
We will now put the proof view handling in the document manager.
Since ther return type for the handle event function is becoming
quite big, we turn it into a record.
Even in continuous mode, observe_id will now indicate where the user clicked.
@rtetley rtetley marked this pull request as ready for review November 28, 2024 08:33
(** [handle_event dpc ev] handles a parsing event for the document. One parsing event parses one line
and prepares the next parsing event. Finally once the full parsing is done, the final event returs
the id of the bottomost sentence of the prefix which has not changed since the previous validation
as well as the set of invalidated sentences. *)
Copy link
Member

Choose a reason for hiding this comment

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

I think you should build a record for the return type, so that we can name all these documents

(** [init st opts uri text] initializes the document manager with initial vernac state
[st] on which command line opts will be set. *)
[st] on which command line opts will be set. [background] indicates if the document should
be executed in the background once it is parsed. *)
Copy link
Member

Choose a reason for hiding this comment

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

there is no background here

val clear_observe_id : state -> state
(** [clear_observe_id state] updates the state to make the observe_id None *)
id is updated. Finally if [background] is set to true, an execution is launched in
the background *)
Copy link
Member

Choose a reason for hiding this comment

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

there is no background here too

(** resets Coq *)

val executed_ranges : state -> exec_overview
val executed_ranges : state -> Settings.Mode.t -> exec_overview
Copy link
Member

Choose a reason for hiding this comment

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

it is unclear what the mode is for here

Copy link
Member

@gares gares left a comment

Choose a reason for hiding this comment

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

looks like a great cleanup

(** [handle_event dpc ev] handles a parsing event for the document. One parsing event parses one line
and prepares the next parsing event. Finally once the full parsing is done, the final event returs
the id of the bottomost sentence of the prefix which has not changed since the previous validation
as well as the set of invalidated sentences. *)
Copy link
Member

Choose a reason for hiding this comment

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

When parsing ends, we still have two documents, hence two cancelation handles. This is a bit confusing: which document should the client keep?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure I catch your drift ? We have one "parsing" document which just became "parsed", and one "previous" document which is the version before all the new parsing events happened.

@rtetley rtetley merged commit 96b677a into main Dec 3, 2024
28 checks passed
@rtetley rtetley deleted the cancel-event branch December 13, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slowdown after editing file Changes appear to be processed character by character (VsCoq2 )
2 participants