From 739cfc03ba9a06d42cf0053e87e40abaf8a3b500 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 23 Nov 2017 23:14:21 -0500 Subject: [PATCH 1/3] Fix undercounting short hints --- qutebrowser/browser/hints.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 8d8a5ae68..b68d00058 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -446,8 +446,17 @@ class HintManager(QObject): # Short hints are the number of hints we can possibly show which are # (needed - 1) digits in length. if needed > min_chars: - short_count = math.floor((len(chars) ** needed - len(elems)) / - len(chars)) + total_space = len(chars) ** needed + # Calculate short_count naively, by finding the avaiable space and + # dividing by the number of spots we would loose by adding a + # short element + short_count = math.floor((total_space - len(elems)) / + (len(chars))) + # Check if we double counted above to warrant another short_count + # https://github.com/qutebrowser/qutebrowser/issues/3242 + if total_space - (short_count * len(chars) + + (len(elems) - short_count)) >= len(chars) - 1: + short_count += 1 else: short_count = 0 From c9af36909f054fff59cfdb0d2a58a441b6149d54 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 24 Nov 2017 13:21:21 -0500 Subject: [PATCH 2/3] Add tests for hint scattering --- tests/end2end/test_hints_html.py | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/end2end/test_hints_html.py b/tests/end2end/test_hints_html.py index abc106505..7b84bd635 100644 --- a/tests/end2end/test_hints_html.py +++ b/tests/end2end/test_hints_html.py @@ -22,12 +22,17 @@ import os import os.path import textwrap +import string +import functools +import operator import attr import yaml import pytest import bs4 +import qutebrowser.browser.hints + def collect_tests(): basedir = os.path.dirname(__file__) @@ -146,3 +151,45 @@ def test_word_hints_issue1393(quteproc, tmpdir): quteproc.wait_for(message='hints: *', category='hints') quteproc.send_cmd(':follow-hint {}'.format(hint)) quteproc.wait_for_load_finished('data/{}'.format(target)) + + +@pytest.mark.parametrize('min_len', [0, 3]) +@pytest.mark.parametrize('num_chars', [9]) +@pytest.mark.parametrize('num_elements', range(1, 26)) +def test_scattered_hints_count(win_registry, mode_manager, min_len, + num_chars, num_elements): + """Test scattered hints function.""" + # pylint: disable=W0141 + manager = qutebrowser.browser.hints.HintManager(0, 0) + chars = string.ascii_lowercase[:num_chars] + + hints = manager._hint_scattered(min_len, chars, + list(range(num_elements))) + + # Check if hints are unique + assert len(hints) == len(set(hints)) + + # Check if any hints are shorter than min_len + assert not any([x for x in hints if len(x) < min_len]) + + # Check we don't have more than 2 link lengths + # Eg: 'a' 'bc' and 'def' cannot be in the same hint string + hint_lens = set(map(len, hints)) + assert len(hint_lens) <= 2 + + if len(hint_lens) == 2: + # Check if hint_lens are more than 1 apart + # Eg: 'abc' and 'd' cannot be in the same hint sequence, but + # 'ab' and 'c' can + assert abs(functools.reduce(operator.sub, hint_lens)) <= 1 + + longest_hints = filter(lambda x: len(x) == max(hint_lens), hints) + + if min_len < max(hint_lens) - 1: + # Check if we have any unique prefixes. For example, 'la' + # alone, with no other 'l' + count_map = {} + for x in longest_hints: + prefix = x[:-1] + count_map[prefix] = count_map.get(prefix, 0) + 1 + assert not any(filter(lambda x: x == 1, count_map.values())) From 19cecbdeaecbe740bab02adeaebd88c2cf4998d9 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Tue, 28 Nov 2017 22:08:22 -0500 Subject: [PATCH 3/3] Fix style issues in scattered hint tests --- tests/end2end/test_hints_html.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/end2end/test_hints_html.py b/tests/end2end/test_hints_html.py index 7b84bd635..5e4ad3837 100644 --- a/tests/end2end/test_hints_html.py +++ b/tests/end2end/test_hints_html.py @@ -158,8 +158,14 @@ def test_word_hints_issue1393(quteproc, tmpdir): @pytest.mark.parametrize('num_elements', range(1, 26)) def test_scattered_hints_count(win_registry, mode_manager, min_len, num_chars, num_elements): - """Test scattered hints function.""" - # pylint: disable=W0141 + """Test scattered hints function. + + Tests many properties from an invocation of _hint_scattered, including + + 1. Hints must be unique + 2. There can only be two hint lengths, only 1 apart + 3. There are no unique prefixes for long hints, such as 'la' with no 'l' + """ manager = qutebrowser.browser.hints.HintManager(0, 0) chars = string.ascii_lowercase[:num_chars] @@ -170,11 +176,11 @@ def test_scattered_hints_count(win_registry, mode_manager, min_len, assert len(hints) == len(set(hints)) # Check if any hints are shorter than min_len - assert not any([x for x in hints if len(x) < min_len]) + assert not any(x for x in hints if len(x) < min_len) # Check we don't have more than 2 link lengths # Eg: 'a' 'bc' and 'def' cannot be in the same hint string - hint_lens = set(map(len, hints)) + hint_lens = {len(h) for h in hints} assert len(hint_lens) <= 2 if len(hint_lens) == 2: @@ -183,7 +189,7 @@ def test_scattered_hints_count(win_registry, mode_manager, min_len, # 'ab' and 'c' can assert abs(functools.reduce(operator.sub, hint_lens)) <= 1 - longest_hints = filter(lambda x: len(x) == max(hint_lens), hints) + longest_hints = [x for x in hints if len(x) == max(hint_lens)] if min_len < max(hint_lens) - 1: # Check if we have any unique prefixes. For example, 'la' @@ -192,4 +198,4 @@ def test_scattered_hints_count(win_registry, mode_manager, min_len, for x in longest_hints: prefix = x[:-1] count_map[prefix] = count_map.get(prefix, 0) + 1 - assert not any(filter(lambda x: x == 1, count_map.values())) + assert all(e != 1 for e in count_map.values())