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
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 51 additions & 30 deletions plugin/save_command.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from .core.registry import LspTextCommand
from .core.settings import userprefs
from .core.typing import Any, Callable, Dict, List, Type
from .core.typing import Any, Callable, Dict, List, Optional, Type
from abc import ABCMeta, abstractmethod
from functools import partial
import sublime
import sublime_plugin

Expand Down Expand Up @@ -60,6 +61,47 @@ def _purge_changes_async(self) -> None:
break


class SaveTasksRunner:
def __init__(
self, text_command: LspTextCommand, tasks: List[Type[SaveTask]], on_complete: Callable[[], None]
) -> None:
self._text_command = text_command
self._tasks = tasks
self._on_tasks_completed = on_complete
self._pending_tasks = [] # type: List[SaveTask]
self._canceled = False

def run(self) -> None:
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.

self._process_next_task()

def cancel(self) -> None:
for task in self._pending_tasks:
task.cancel()
self._pending_tasks = []
self._canceled = True

def _process_next_task(self) -> None:
if self._pending_tasks:
# Even though we might be on an async thread already, we want to give ST a chance to notify us about
# potential document changes.
sublime.set_timeout_async(self._run_next_task_async)
else:
self._on_tasks_completed()

def _run_next_task_async(self) -> None:
if self._canceled:
return
current_task = self._pending_tasks[0]
current_task.run_async()

def _on_task_completed_async(self) -> None:
self._pending_tasks.pop(0)
self._process_next_task()


class LspSaveCommand(LspTextCommand):
"""
A command used as a substitute for native save command. Runs code actions and document
Expand All @@ -74,23 +116,14 @@ def register_task(cls, task: Type[SaveTask]) -> None:

def __init__(self, view: sublime.View) -> None:
super().__init__(view)
self._pending_tasks = [] # type: List[SaveTask]
self._kwargs = {} # type: Dict[str, Any]
self._save_tasks_runner = None # type: Optional[SaveTasksRunner]

def run(self, edit: sublime.Edit, **kwargs: Dict[str, Any]) -> None:
if self._pending_tasks:
for task in self._pending_tasks:
task.cancel()
self._pending_tasks = []
self._kwargs = kwargs
if self._save_tasks_runner:
self._save_tasks_runner.cancel()
sublime.set_timeout_async(self._trigger_on_pre_save_async)
for Task in self._tasks:
if Task.is_applicable(self.view):
self._pending_tasks.append(Task(self, self._on_task_completed_async))
if self._pending_tasks:
sublime.set_timeout_async(self._run_next_task_async)
else:
self._trigger_native_save()
self._save_tasks_runner = SaveTasksRunner(self, self._tasks, partial(self._on_tasks_completed, kwargs))
self._save_tasks_runner.run()

def _trigger_on_pre_save_async(self) -> None:
# Supermassive hack that will go away later.
Expand All @@ -100,22 +133,10 @@ def _trigger_on_pre_save_async(self) -> None:
listener.trigger_on_pre_save_async() # type: ignore
break

def _run_next_task_async(self) -> None:
current_task = self._pending_tasks[0]
current_task.run_async()

def _on_task_completed_async(self) -> None:
self._pending_tasks.pop(0)
if self._pending_tasks:
# Even though we are on the async thread already, we want to give ST a chance to notify us about
# potential document changes.
sublime.set_timeout_async(self._run_next_task_async)
else:
self._trigger_native_save()

def _trigger_native_save(self) -> None:
def _on_tasks_completed(self, kwargs: Dict[str, Any]) -> None:
self._save_tasks_runner = None
# Triggered from set_timeout to preserve original semantics of on_pre_save handling
sublime.set_timeout(lambda: self.view.run_command('save', self._kwargs))
sublime.set_timeout(lambda: self.view.run_command('save', kwargs))


class LspSaveAllCommand(sublime_plugin.WindowCommand):
Expand Down