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

refactor task running on save #2384

Merged
merged 3 commits into from
Jan 3, 2024
Merged

refactor task running on save #2384

merged 3 commits into from
Jan 3, 2024

Conversation

rchl
Copy link
Member

@rchl rchl commented Dec 28, 2023

Refactor the code that runs tasks on saving to handle cancellation properly when running lsp_save command twice in a row, in a sync fashion.

Previously the queued _run_next_task_async callback from the first invocation still triggered after the first invocation was canceled, triggering an exception.

Now the tasks running logic is encapsulated into its own class, making cancelling easier and more robust.

Comment on lines 75 to 77
for Task in self._tasks:
if Task.is_applicable(self._text_command.view):
self._pending_tasks.append(Task(self._text_command, self._on_task_completed_async))
Copy link
Member

@jwortmann jwortmann Jan 2, 2024

Choose a reason for hiding this comment

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

It wasn't changed in this PR, but the uppercase Task is a bit inconsistent with the lines 113-115 in this file, where it is lowercase. I don't know what the convention is (or if a convention exists for it), but I would say it should either be lowercase here or uppercase in lines 113-115.

In __init__ the tasks parameter is also lowercase.

@rchl rchl merged commit 4ecaca8 into main Jan 3, 2024
4 checks passed
@rchl rchl deleted the fix/save-task-refactor branch January 3, 2024 12:46
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.

2 participants