From 8eb12c6cb9d89a19c8ba25b0dde5874f94e75035 Mon Sep 17 00:00:00 2001 From: Daniel Karbach Date: Thu, 29 Sep 2016 13:29:39 +0200 Subject: [PATCH 1/6] improved error messages for inexistent userscripts fixes #1940 --- qutebrowser/browser/commands.py | 2 +- qutebrowser/commands/userscripts.py | 28 ++++++++++++++++++++++++++++ tests/end2end/features/spawn.feature | 6 +++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 5e899aba1..a06fc9361 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1122,7 +1122,7 @@ class CommandDispatcher: try: userscripts.run_async(tab, cmd, *args, win_id=self._win_id, env=env, verbose=verbose) - except userscripts.UnsupportedError as e: + except (userscripts.UnsupportedError, userscripts.NotFoundError) as e: raise cmdexc.CommandError(e) @cmdutils.register(instance='command-dispatcher', scope='window') diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 4601ed528..1cb4f14f4 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -315,6 +315,27 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): return +class NotFoundError(Exception): + + """Raised when spawning a userscript that doesn't exist. + + Attributes: + script_name: name of the userscript as called + paths: path names that were searched for the userscript + message: descriptive message string of the error + """ + + def __init__(self, script_name, paths=[]): + self.script_name = script_name + self.paths = paths + self.message = "Userscript '" + script_name + "' not found" + if paths: + self.message += " in userscript directory '" + ("' or '".join(paths)) + "'" + + def __str__(self): + return self.message + + class UnsupportedError(Exception): """Raised when userscripts aren't supported on this platform.""" @@ -375,6 +396,13 @@ def run_async(tab, cmd, *args, win_id, env, verbose=False): if not os.path.exists(cmd_path): cmd_path = os.path.join(standarddir.system_data(), "userscripts", cmd) + if not os.path.exists(cmd_path): + raise NotFoundError(cmd, [ + os.path.join(standarddir.data(), "userscripts"), + os.path.join(standarddir.system_data(), "userscripts"), + ]) + elif not os.path.exists(cmd_path): + raise NotFoundError(cmd_path) log.misc.debug("Userscript to run: {}".format(cmd_path)) runner.finished.connect(commandrunner.deleteLater) diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index 5cdeac029..341e6c921 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -10,7 +10,11 @@ Feature: :spawn Scenario: Starting a userscript which doesn't exist When I run :spawn -u this_does_not_exist - Then the error "Error while spawning userscript: The process failed to start." should be shown + Then regex "Userscript 'this_does_not_exist' not found in userscript directory '(.*)'( or '(.*)')*" should be logged + + Scenario: Starting a userscript with absoloute path which doesn't exist + When I run :spawn -u /this_does_not_exist + Then the error "Userscript '/this_does_not_exist' not found" should be shown # https://github.com/The-Compiler/qutebrowser/issues/1614 @posix From cbbfbabfc4906a741bc03247b19ad899fc8fb5d1 Mon Sep 17 00:00:00 2001 From: Daniel Karbach Date: Thu, 29 Sep 2016 14:37:40 +0200 Subject: [PATCH 2/6] reorganize and document --- qutebrowser/commands/userscripts.py | 45 ++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 1cb4f14f4..34898ad52 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -325,12 +325,14 @@ class NotFoundError(Exception): message: descriptive message string of the error """ - def __init__(self, script_name, paths=[]): + def __init__(self, script_name, paths=None): + super().__init__() self.script_name = script_name self.paths = paths self.message = "Userscript '" + script_name + "' not found" if paths: - self.message += " in userscript directory '" + ("' or '".join(paths)) + "'" + self.message += " in userscript directory '" + \ + ("' or '".join(paths)) + "'" def __str__(self): return self.message @@ -344,12 +346,41 @@ class UnsupportedError(Exception): return "Userscripts are not supported on this platform!" +def _lookup_path(cmd): + """Search userscript directories for given command. + + Raises: + NotFoundError if the command could not be found. + + Args: + cmd: The command to look for. + + Returns: + A path to the userscript. + """ + cmd_path = os.path.join(standarddir.data(), "userscripts", cmd) + if os.path.exists(cmd_path): + return cmd_path + + directories = [ + os.path.join(standarddir.data(), "userscripts"), + os.path.join(standarddir.system_data(), "userscripts"), + ] + for directory in directories: + cmd_path = os.path.join(directory, cmd) + if os.path.exists(cmd_path): + return cmd_path + + raise NotFoundError(cmd, directories) + + def run_async(tab, cmd, *args, win_id, env, verbose=False): """Run a userscript after dumping page html/source. Raises: UnsupportedError if userscripts are not supported on the current platform. + NotFoundError if the command could not be found. Args: tab: The WebKitTab/WebEngineTab to get the source from. @@ -392,15 +423,7 @@ def run_async(tab, cmd, *args, win_id, env, verbose=False): # ~/.local/share/qutebrowser/userscripts (or $XDG_DATA_DIR) if not os.path.isabs(cmd_path): log.misc.debug("{} is no absolute path".format(cmd_path)) - cmd_path = os.path.join(standarddir.data(), "userscripts", cmd) - if not os.path.exists(cmd_path): - cmd_path = os.path.join(standarddir.system_data(), - "userscripts", cmd) - if not os.path.exists(cmd_path): - raise NotFoundError(cmd, [ - os.path.join(standarddir.data(), "userscripts"), - os.path.join(standarddir.system_data(), "userscripts"), - ]) + cmd_path = _lookup_path(cmd) elif not os.path.exists(cmd_path): raise NotFoundError(cmd_path) log.misc.debug("Userscript to run: {}".format(cmd_path)) From eaa754648d6fdb458614e1518ee7616475ef5b65 Mon Sep 17 00:00:00 2001 From: Daniel Karbach Date: Fri, 30 Sep 2016 09:26:43 +0200 Subject: [PATCH 3/6] common base for userscript exceptions --- qutebrowser/browser/commands.py | 2 +- qutebrowser/commands/userscripts.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index a06fc9361..4be7ed583 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1122,7 +1122,7 @@ class CommandDispatcher: try: userscripts.run_async(tab, cmd, *args, win_id=self._win_id, env=env, verbose=verbose) - except (userscripts.UnsupportedError, userscripts.NotFoundError) as e: + except userscripts.Error as e: raise cmdexc.CommandError(e) @cmdutils.register(instance='command-dispatcher', scope='window') diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 34898ad52..2d9fb1dd1 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -315,7 +315,12 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): return -class NotFoundError(Exception): +class Error(Exception): + + """Base class for userscript exceptions.""" + + +class NotFoundError(Error): """Raised when spawning a userscript that doesn't exist. @@ -338,7 +343,7 @@ class NotFoundError(Exception): return self.message -class UnsupportedError(Exception): +class UnsupportedError(Error): """Raised when userscripts aren't supported on this platform.""" From dc344989c6e05550206c34cbf66196aa9615e3ca Mon Sep 17 00:00:00 2001 From: Daniel Karbach Date: Fri, 30 Sep 2016 09:39:14 +0200 Subject: [PATCH 4/6] revised missing userscript error messages --- qutebrowser/commands/userscripts.py | 11 +++++------ tests/end2end/features/spawn.feature | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 2d9fb1dd1..eb2de03ed 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -327,20 +327,19 @@ class NotFoundError(Error): Attributes: script_name: name of the userscript as called paths: path names that were searched for the userscript - message: descriptive message string of the error """ def __init__(self, script_name, paths=None): super().__init__() self.script_name = script_name self.paths = paths - self.message = "Userscript '" + script_name + "' not found" - if paths: - self.message += " in userscript directory '" + \ - ("' or '".join(paths)) + "'" def __str__(self): - return self.message + msg = "Userscript '{}' not found".format(self.script_name) + if self.paths: + msg += " in userscript directories {}".format( + ', '.join(repr(path) for path in self.paths)) + return msg class UnsupportedError(Error): diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index 341e6c921..dc12f4078 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -10,7 +10,7 @@ Feature: :spawn Scenario: Starting a userscript which doesn't exist When I run :spawn -u this_does_not_exist - Then regex "Userscript 'this_does_not_exist' not found in userscript directory '(.*)'( or '(.*)')*" should be logged + Then the error "Userscript 'this_does_not_exist' not found in userscript directories *" should be shown Scenario: Starting a userscript with absoloute path which doesn't exist When I run :spawn -u /this_does_not_exist From a3e9fe1fd7632df6387f859da3ebdabb3b5f640c Mon Sep 17 00:00:00 2001 From: Daniel Karbach Date: Fri, 30 Sep 2016 09:41:17 +0200 Subject: [PATCH 5/6] removed duplicate userscript path lookup --- qutebrowser/commands/userscripts.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index eb2de03ed..de557f940 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -362,10 +362,6 @@ def _lookup_path(cmd): Returns: A path to the userscript. """ - cmd_path = os.path.join(standarddir.data(), "userscripts", cmd) - if os.path.exists(cmd_path): - return cmd_path - directories = [ os.path.join(standarddir.data(), "userscripts"), os.path.join(standarddir.system_data(), "userscripts"), From 3ffbf07eab86dea2ea02e714750b5f7c5c4c5c85 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 3 Oct 2016 06:42:13 +0200 Subject: [PATCH 6/6] Update docs --- CHANGELOG.asciidoc | 1 + README.asciidoc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 2c790010c..051f86487 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -136,6 +136,7 @@ Changed * `fonts -> messages.info` - The `qute:settings` page now also shows option descriptions. - `qute:version` and `qutebrowser --version` now show various important paths +- `:spawn`/userscripts now show a nicer error when a script wasn't found Deprecated ~~~~~~~~~~ diff --git a/README.asciidoc b/README.asciidoc index 6eea6948b..fc710ac98 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -166,9 +166,9 @@ Contributors, sorted by the number of commits in descending order: * Nathan Isom * Thorsten Wißmann * Austin Anderson +* Daniel Karbach * Jimmy * Niklas Haas -* Daniel Karbach * Alexey "Averrin" Nabrodov * nanjekyejoannah * avk