From 822c100f524f97ba5ff3648cafbdf81a27b2ee34 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 30 Sep 2016 08:33:16 +0200 Subject: [PATCH 1/5] Make 0 a usable count for :tab-focus Fixes #1768 --- qutebrowser/browser/commands.py | 8 ++++++-- qutebrowser/commands/command.py | 22 ++++++++++++++++++---- qutebrowser/keyinput/basekeyparser.py | 3 --- tests/end2end/features/tabs.feature | 11 +++++++++++ tests/unit/commands/test_cmdutils.py | 11 +++++++++++ tests/unit/keyinput/test_basekeyparser.py | 10 +++++----- 6 files changed, 51 insertions(+), 14 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 5e899aba1..f3f12acba 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -956,7 +956,7 @@ class CommandDispatcher: @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('index', choices=['last']) - @cmdutils.argument('count', count=True) + @cmdutils.argument('count', count=True, zero_count=True) def tab_focus(self, index: typing.Union[str, int]=None, count=None): """Select the tab given as argument/[count]. @@ -969,6 +969,7 @@ class CommandDispatcher: Negative indices count from the end, such that -1 is the last tab. count: The tab index to focus, starting with 1. + The special value 0 focuses the rightmost tab. """ if index == 'last': self._tab_focus_last() @@ -977,7 +978,10 @@ class CommandDispatcher: if index is None: self.tab_next() return - if index < 0: + elif index == 0: + index = self._count() + + elif index < 0: index = self._count() + index + 1 if 1 <= index <= self._count(): diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index d899a543f..5d98c9166 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -33,12 +33,15 @@ class ArgInfo: """Information about an argument.""" - def __init__(self, win_id=False, count=False, flag=None, hide=False, - metavar=None, completion=None, choices=None): + def __init__(self, win_id=False, count=False, hide=False, metavar=None, + zero_count=False, flag=None, completion=None, choices=None): if win_id and count: raise TypeError("Argument marked as both count/win_id!") + if zero_count and not count: + raise TypeError("zero_count argument cannot exist without count!") self.win_id = win_id self.count = count + self.zero_count = zero_count self.flag = flag self.hide = hide self.metavar = metavar @@ -48,6 +51,7 @@ class ArgInfo: def __eq__(self, other): return (self.win_id == other.win_id and self.count == other.count and + self.zero_count == other.zero_count and self.flag == other.flag and self.hide == other.hide and self.metavar == other.metavar and @@ -57,6 +61,7 @@ class ArgInfo: def __repr__(self): return utils.get_repr(self, win_id=self.win_id, count=self.count, flag=self.flag, hide=self.hide, + zero_count=self.zero_count, metavar=self.metavar, completion=self.completion, choices=self.choices, constructor=True) @@ -137,6 +142,7 @@ class Command: self.opt_args = collections.OrderedDict() self.namespace = None self._count = None + self._zero_count = None self.pos_args = [] self.desc = None self.flags_with_args = [] @@ -148,7 +154,7 @@ class Command: self._inspect_func() - def _check_prerequisites(self, win_id): + def _check_prerequisites(self, win_id, count): """Check if the command is permitted to run currently. Args: @@ -164,6 +170,11 @@ class Command: "{}: Only available with {} " "backend.".format(self.name, self.backend.name)) + if count == 0 and not self._zero_count: + raise cmdexc.PrerequisitesError( + "{}: A zero count is not allowed for this command!" + .format(self.name)) + if self.deprecated: message.warning('{} is deprecated - {}'.format(self.name, self.deprecated)) @@ -235,6 +246,9 @@ class Command: assert param.kind != inspect.Parameter.POSITIONAL_ONLY if param.name == 'self': continue + arg_info = self.get_arg_info(param) + if arg_info.count: + self._zero_count = arg_info.zero_count if self._inspect_special_param(param): continue if (param.kind == inspect.Parameter.KEYWORD_ONLY and @@ -511,7 +525,7 @@ class Command: e.status, e)) return self._count = count - self._check_prerequisites(win_id) + self._check_prerequisites(win_id, count) posargs, kwargs = self._get_call_args(win_id) log.commands.debug('Calling {}'.format( debug_utils.format_call(self.handler, posargs, kwargs))) diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index 8578229be..59ac51a60 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -146,9 +146,6 @@ class BaseKeyParser(QObject): (countstr, cmd_input) = re.match(r'^(\d*)(.*)', self._keystring).groups() count = int(countstr) if countstr else None - if count == 0 and not cmd_input: - cmd_input = self._keystring - count = None else: cmd_input = self._keystring count = None diff --git a/tests/end2end/features/tabs.feature b/tests/end2end/features/tabs.feature index 7f7f9cdcb..7ae98619c 100644 --- a/tests/end2end/features/tabs.feature +++ b/tests/end2end/features/tabs.feature @@ -255,6 +255,17 @@ Feature: Tab management - data/numbers/2.txt (active) - data/numbers/3.txt + Scenario: :tab-focus with count 0 + When I open data/numbers/1.txt + And I open data/numbers/2.txt in a new tab + And I open data/numbers/3.txt in a new tab + And I run :tab-focus with count 1 + And I run :tab-focus with count 0 + Then the following tabs should be open: + - data/numbers/1.txt + - data/numbers/2.txt + - data/numbers/3.txt (active) + Scenario: :tab-focus with invalid negative index When I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index 28e7d5fed..264da8087 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -423,6 +423,17 @@ class TestArgument: assert str(excinfo.value) == "Argument marked as both count/win_id!" + def test_count_and_zero_count_arg(self): + with pytest.raises(TypeError) as excinfo: + @cmdutils.argument('arg', count=False, zero_count=True) + def fun(arg=0): + """Blah.""" + pass + + expected = "zero_count argument cannot exist without count!" + + assert str(excinfo.value) == expected + def test_no_docstring(self, caplog): with caplog.at_level(logging.WARNING): @cmdutils.register() diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index da1ecfbdf..0da7dba24 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -246,9 +246,10 @@ class TestKeyChain: assert keyparser._keystring == '' def test_0_press(self, handle_text, keyparser): - handle_text((Qt.Key_0, '0')) + handle_text((Qt.Key_X, '0'), + (Qt.Key_B, 'b'), (Qt.Key_A, 'a')) keyparser.execute.assert_called_once_with( - '0', keyparser.Type.chain, None) + 'ba', keyparser.Type.chain, 0) assert keyparser._keystring == '' def test_ambiguous_keychain(self, qapp, handle_text, config_stub, @@ -314,9 +315,8 @@ class TestCount: def test_count_0(self, handle_text, keyparser): handle_text((Qt.Key_0, '0'), (Qt.Key_B, 'b'), (Qt.Key_A, 'a')) - calls = [mock.call('0', keyparser.Type.chain, None), - mock.call('ba', keyparser.Type.chain, None)] - keyparser.execute.assert_has_calls(calls) + keyparser.execute.assert_called_once_with( + 'ba', keyparser.Type.chain, 0) assert keyparser._keystring == '' def test_count_42(self, handle_text, keyparser): From 6b76d5defa639e6222a8c2e902a61834f25c79b2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 30 Sep 2016 17:54:23 +0200 Subject: [PATCH 2/5] Whitespace changes --- qutebrowser/browser/commands.py | 2 +- tests/unit/commands/test_cmdutils.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f3f12acba..b6e4a2211 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -975,12 +975,12 @@ class CommandDispatcher: self._tab_focus_last() return index = count if count is not None else index + if index is None: self.tab_next() return elif index == 0: index = self._count() - elif index < 0: index = self._count() + index + 1 diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index 264da8087..aaf63014e 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -431,7 +431,6 @@ class TestArgument: pass expected = "zero_count argument cannot exist without count!" - assert str(excinfo.value) == expected def test_no_docstring(self, caplog): From 083d84731627b81bbcab0e2b7979c0399359ea90 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 30 Sep 2016 18:14:22 +0200 Subject: [PATCH 3/5] Add a test for a zero count --- tests/end2end/features/misc.feature | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 8a4c3f09e..c24f0ea9f 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -604,6 +604,10 @@ Feature: Various utility commands. - data/hints/link_blank.html - data/hello.txt (active) + Scenario: Using 0 as count + When I run :scroll down with count 0 + Then the error "scroll: A zero count is not allowed for this command!" should be shown + @no_xvfb Scenario: :window-only Given I run :tab-only From 28c87b5c6b5817f9e5db11492d0bcd2f44da6498 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 30 Sep 2016 18:15:59 +0200 Subject: [PATCH 4/5] Use right key in test_basekeyparser --- tests/unit/keyinput/test_basekeyparser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 0da7dba24..4973954b3 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -246,7 +246,7 @@ class TestKeyChain: assert keyparser._keystring == '' def test_0_press(self, handle_text, keyparser): - handle_text((Qt.Key_X, '0'), + handle_text((Qt.Key_0, '0'), (Qt.Key_B, 'b'), (Qt.Key_A, 'a')) keyparser.execute.assert_called_once_with( 'ba', keyparser.Type.chain, 0) From c80f18522c11396a1551569b6cae11eae844ba22 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 30 Sep 2016 18:17:10 +0200 Subject: [PATCH 5/5] Remove now-duplicate test TestKeychain.test_count_0 now tests the exact same thing. --- tests/unit/keyinput/test_basekeyparser.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 4973954b3..3e55d0366 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -245,13 +245,6 @@ class TestKeyChain: 'ba', keyparser.Type.chain, None) assert keyparser._keystring == '' - def test_0_press(self, handle_text, keyparser): - handle_text((Qt.Key_0, '0'), - (Qt.Key_B, 'b'), (Qt.Key_A, 'a')) - keyparser.execute.assert_called_once_with( - 'ba', keyparser.Type.chain, 0) - assert keyparser._keystring == '' - def test_ambiguous_keychain(self, qapp, handle_text, config_stub, keyparser): config_stub.data = CONFIG