From c6f790c35cc935f5be5bf00ff38f4f820d354cf6 Mon Sep 17 00:00:00 2001 From: Andrew Hamilton Date: Mon, 9 Mar 2020 14:31:01 +1000 Subject: [PATCH] Optimize change to summary when filesystem changes. - Was checking all files on any change detected. - Now adding, deleting or modifing individual rows in response to the respective filesystem event. - html_syntax tool was being run in an infinite loop because it triggered the ON_CLOSE_WRITE event. --- TODO | 2 +- eris/__main__.py | 92 +++++++++++++++++++++++++++++++++++++----- eris/tools.py | 10 +++-- tests/__main___test.py | 51 +++++++++++++++++------ 4 files changed, 127 insertions(+), 28 deletions(-) diff --git a/TODO b/TODO index 40ee979..53c90de 100644 --- a/TODO +++ b/TODO @@ -7,6 +7,7 @@ Todo Todo (tool related) - Report on python doctests. (also coverage of) +- eslint for javascript (at least for js,md,ts) Done @@ -220,7 +221,6 @@ Ideas csv file? - Check these tools: astyle, indent, uncrustify, xmlindent, csstidy, flake8, frosted, pep257, pyroma, dodgy, jedi, pep8-naming, graphite, propector, vmprof, pytype -- eslint for javascript? - epydoc for python - readelf - pinfer from mypy diff --git a/eris/__main__.py b/eris/__main__.py index 89621dd..db4913a 100755 --- a/eris/__main__.py +++ b/eris/__main__.py @@ -95,6 +95,9 @@ class Entry: self.widget = fill3.Row(results) self.appearance_cache = None + def __eq__(self, other): + return self.path == other.path + def __len__(self): return len(self.widgets) @@ -257,6 +260,65 @@ class Summary: else type_sort) self.closest_placeholder_generator = None + def file_added(self, path): + full_path = os.path.join(self._root_path, path) + try: + file_key = (path, os.stat(full_path).st_ctime) + except FileNotFoundError: + return + row = [] + for tool in tools.tools_for_path(path): + tool_key = (tool.__name__, tool.__code__.co_code) + result = tools.Result(path, tool) + self._all_results.add(result) + self.result_total += 1 + file_entry = self._cache.setdefault(file_key, {}) + file_entry[tool_key] = result + if result.is_completed: + self.completed_total += 1 + row.append(result) + self._max_width = max(len(row), self._max_width) + self._max_path_length = max(len(path) - len("./"), + self._max_path_length) + self._column.append(Entry(path, row, self)) + self.sort_entries() + self._jobs_added_event.set() + self.closest_placeholder_generator = None + + def file_deleted(self, path): + entry = Entry(path, [], self) + try: + index = self._column.index(entry) + except ValueError: + return + new_cache = {} + for file_key in self._cache: + cache_path, cache_time = file_key + if cache_path != path: + new_cache[file_key] = self._cache[file_key] + self._cache = new_cache + for result in self._column[index]: + self._all_results.remove(result) + if result.is_completed: + self.completed_total -= 1 + self.result_total -= 1 + result.delete() + row = self._column[index] + del self._column[index] + if len(row) == self._max_width: + self._max_width = max(len(entry) for entry in self._column) + if (len(path) - 2) == self._max_path_length: + self._max_path_length = max((len(entry.path) - 2) + for entry in self._column) + x, y = self._cursor_position + if y == len(self._column): + self._cursor_position = x, y - 1 + self.closest_placeholder_generator = None + + def file_modified(self, path): + self.file_deleted(path) + self.file_added(path) + @contextlib.contextmanager def keep_selection(self): try: @@ -1012,16 +1074,16 @@ class Screen: ({"tab"}, toggle_focus), ({"f"}, toggle_fullscreen), ("x", xdg_open)] -def setup_inotify(root_path, loop, on_filesystem_change, exclude_filter): +def setup_inotify(root_path, loop, on_filesystem_event, exclude_filter): watch_manager = pyinotify.WatchManager() event_mask = (pyinotify.IN_CREATE | pyinotify.IN_DELETE | pyinotify.IN_CLOSE_WRITE | pyinotify.IN_ATTRIB | pyinotify.IN_MOVED_FROM | pyinotify.IN_MOVED_TO) watch_manager.add_watch(root_path, event_mask, rec=True, auto_add=True, - proc_fun=lambda event: None, + proc_fun=on_filesystem_event, exclude_filter=exclude_filter) return pyinotify.AsyncioNotifier(watch_manager, loop, - callback=on_filesystem_change) + callback=lambda notifier: None) def load_state(pickle_path, jobs_added_event, appearance_changed_event, @@ -1047,6 +1109,20 @@ def load_state(pickle_path, jobs_added_event, appearance_changed_event, return summary, screen, log, is_first_run +def on_filesystem_event(event, summary, root_path, appearance_changed_event): + path = fix_paths(root_path, [event.pathname])[0] + if is_path_excluded(path[2:]): + return + inotify_actions = {pyinotify.IN_CREATE: summary.file_added, + pyinotify.IN_MOVED_TO: summary.file_added, + pyinotify.IN_DELETE: summary.file_deleted, + pyinotify.IN_MOVED_FROM: summary.file_deleted, + pyinotify.IN_ATTRIB: summary.file_modified, + pyinotify.IN_CLOSE_WRITE: summary.file_modified} + inotify_actions[event.mask](path) + appearance_changed_event.set() + + def main(root_path, loop, worker_count=None, editor_command=None, theme=None, compression=None, is_being_tested=False): if worker_count is None: @@ -1068,13 +1144,9 @@ def main(root_path, loop, worker_count=None, editor_command=None, theme=None, jobs_added_event.set() if not is_first_run: summary.sync_with_filesystem(log) - - def on_filesystem_change(notifier): - time.sleep(0.1) # A little time for more events - summary.sync_with_filesystem(log) - appearance_changed_event.set() - notifier = setup_inotify(root_path, loop, on_filesystem_change, - is_path_excluded) + callback = lambda event: on_filesystem_event(event, summary, root_path, + appearance_changed_event) + notifier = setup_inotify(root_path, loop, callback, is_path_excluded) try: log.log_message(f"Starting workers ({worker_count}) …") screen.make_workers(worker_count, is_being_tested, compression) diff --git a/eris/tools.py b/eris/tools.py index a7c750d..7aa3d79 100755 --- a/eris/tools.py +++ b/eris/tools.py @@ -14,6 +14,7 @@ import os import os.path import pickle import pwd +import shlex import shutil import stat import subprocess @@ -383,10 +384,11 @@ def perltidy(path): @deps(deps={"tidy"}, url="tidy", executables={"tidy"}) def html_syntax(path): - # Maybe only show errors - stdout, stderr, returncode = _do_command(["tidy", path]) - status = Status.ok if returncode == 0 else Status.problem - return status, stderr + # Stop tidy from modifiying input path by piping in input. + tidy_process = subprocess.run(f"cat {shlex.quote(path)} | tidy", + capture_output=True, text=True, shell=True) + status = Status.ok if tidy_process.returncode == 0 else Status.problem + return status, _fix_input(tidy_process.stderr) @deps(deps={"pandoc"}, url="pandoc", executables={"pandoc"}) diff --git a/tests/__main___test.py b/tests/__main___test.py index 89e7555..9a89d66 100755 --- a/tests/__main___test.py +++ b/tests/__main___test.py @@ -117,42 +117,66 @@ class SummaryCursorTest(unittest.TestCase): (self.summary.cursor_down, (2, 2))]) -class SummarySyncWithFilesystem(unittest.TestCase): +class SummarySyncWithFilesystemTestCase(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() self.foo_path = os.path.join(self.temp_dir, "foo") - self.bar_path = os.path.join(self.temp_dir, "bar") - self.zoo_path = os.path.join(self.temp_dir, "zoo") + self.bar_path = os.path.join(self.temp_dir, "bar.md") + self.zoo_path = os.path.join(self.temp_dir, "zoo.html") _touch(self.foo_path) _touch(self.bar_path) self.jobs_added_event = asyncio.Event() self.appearance_changed_event = asyncio.Event() self.summary = __main__.Summary(self.temp_dir, self.jobs_added_event) self.jobs_added_event.clear() + self.loop = asyncio.new_event_loop() + callback = lambda event: __main__.on_filesystem_event( + event, self.summary, self.temp_dir, self.appearance_changed_event) + __main__.setup_inotify(self.temp_dir, self.loop, callback, + __main__.is_path_excluded) def tearDown(self): shutil.rmtree(self.temp_dir) def _assert_paths(self, expected_paths): actual_paths = [entry[0].path for entry in self.summary._column] - self.assertEqual(actual_paths, expected_paths) + self.assertEqual(set(actual_paths), set(expected_paths)) def test_summary_initial_state(self): - self._assert_paths(["./bar", "./foo"]) + self._assert_paths(["./bar.md", "./foo"]) self.assertFalse(self.jobs_added_event.is_set()) def test_sync_removed_file(self): - os.remove(self.foo_path) - self._assert_paths(["./bar", "./foo"]) - self.summary.sync_with_filesystem() - self._assert_paths(["./bar"]) + self._assert_paths(["./bar.md", "./foo"]) + self.assertEqual(self.summary.result_total, 9) + self.assertEqual(self.summary.completed_total, 0) + self.assertEqual(self.summary._max_width, 5) + self.assertEqual(self.summary._max_path_length, len("bar.md")) + async def foo(): + os.remove(self.bar_path) + self.loop.run_until_complete(foo()) + self._assert_paths(["./foo"]) + self.assertEqual(self.summary.result_total, 4) + self.assertEqual(self.summary.completed_total, 0) + self.assertEqual(self.summary._max_width, 4) + self.assertEqual(self.summary._max_path_length, len("foo")) self.assertFalse(self.jobs_added_event.is_set()) def test_sync_added_file(self): - _touch(self.zoo_path) - self.summary.sync_with_filesystem() - self._assert_paths(["./bar", "./foo", "./zoo"]) + self._assert_paths(["./bar.md", "./foo"]) + self.assertEqual(self.summary.result_total, 9) + self.assertEqual(self.summary.completed_total, 0) + self.assertEqual(self.summary._max_width, 5) + self.assertEqual(self.summary._max_path_length, 6) + async def foo(): + _touch(self.zoo_path) + self.loop.run_until_complete(foo()) + self._assert_paths(["./bar.md", "./foo", "./zoo.html"]) + self.assertEqual(self.summary.result_total, 16) + self.assertEqual(self.summary.completed_total, 0) + self.assertEqual(self.summary._max_width, 7) + self.assertEqual(self.summary._max_path_length, len("zoo.html")) self.assertTrue(self.jobs_added_event.is_set()) # def test_sync_changed_file_metadata(self): @@ -178,7 +202,7 @@ class SummarySyncWithFilesystem(unittest.TestCase): os.symlink(self.foo_path, baz_path) os.link(self.foo_path, self.zoo_path) self.summary.sync_with_filesystem() - self._assert_paths(["./bar", "./baz", "./foo", "./zoo"]) + self._assert_paths(["./bar.md", "./baz", "./foo", "./zoo.html"]) self.assertTrue(id(self.summary._column[1]) != # baz id(self.summary._column[2])) # foo self.assertTrue(id(self.summary._column[2]) != # foo @@ -238,6 +262,7 @@ class MainTestCase(unittest.TestCase): second_dir = os.path.join(temp_dir, "second") os.rename(first_dir, second_dir) test_run(second_dir, loop) + loop.close() loop.stop() finally: shutil.rmtree(temp_dir)