diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 3c4ea0b43..1e2b46be6 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -21,7 +21,6 @@ import collections import functools -import math import os import re import html @@ -457,21 +456,15 @@ class HintManager(QObject): # Determine how many digits the link hints will require in the worst # case. Usually we do not need all of these digits for every link # single hint, so we can show shorter hints for a few of the links. - needed = max(min_chars, math.ceil(math.log(len(elems), len(chars)))) + needed = max(min_chars, utils.ceil_log(len(elems), len(chars))) + # Short hints are the number of hints we can possibly show which are # (needed - 1) digits in length. - if needed > min_chars: + if needed > min_chars and needed > 1: 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 + # For each 1 short link being added, len(chars) long links are + # removed, therefore the space removed is len(chars) - 1. + short_count = (total_space - len(elems)) // (len(chars) - 1) else: short_count = 0 @@ -498,7 +491,7 @@ class HintManager(QObject): elems: The elements to generate labels for. """ strings = [] - needed = max(min_chars, math.ceil(math.log(len(elems), len(chars)))) + needed = max(min_chars, utils.ceil_log(len(elems), len(chars))) for i in range(len(elems)): strings.append(self._number_to_hint_str(i, chars, needed)) return strings diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index f2e19a743..a6c8e866c 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -712,3 +712,16 @@ def guess_mimetype(filename, fallback=False): else: raise ValueError("Got None mimetype for {}".format(filename)) return mimetype + + +def ceil_log(number, base): + """Compute max(1, ceil(log(number, base))). + + Use only integer arithmetic in order to avoid numerical error. + """ + result = 1 + accum = base + while accum < number: + result += 1 + accum *= base + return result diff --git a/tests/unit/browser/test_hints.py b/tests/unit/browser/test_hints.py index 8aa74274a..69612df64 100644 --- a/tests/unit/browser/test_hints.py +++ b/tests/unit/browser/test_hints.py @@ -19,6 +19,7 @@ import string import functools +import itertools import operator import pytest @@ -88,8 +89,8 @@ def test_match_benchmark(benchmark, tabbed_browser, qtbot, message_bridge, @pytest.mark.parametrize('min_len', [0, 3]) -@pytest.mark.parametrize('num_chars', [9]) -@pytest.mark.parametrize('num_elements', range(1, 26)) +@pytest.mark.parametrize('num_chars', [5, 9]) +@pytest.mark.parametrize('num_elements', itertools.chain(range(1, 26), [125])) def test_scattered_hints_count(min_len, num_chars, num_elements): """Test scattered hints function. @@ -122,7 +123,9 @@ def test_scattered_hints_count(min_len, num_chars, num_elements): # 'ab' and 'c' can assert abs(functools.reduce(operator.sub, hint_lens)) <= 1 - longest_hints = [x for x in hints if len(x) == max(hint_lens)] + longest_hint_len = max(hint_lens) + shortest_hint_len = min(hint_lens) + longest_hints = [x for x in hints if len(x) == longest_hint_len] if min_len < max(hint_lens) - 1: # Check if we have any unique prefixes. For example, 'la' @@ -132,3 +135,18 @@ def test_scattered_hints_count(min_len, num_chars, num_elements): prefix = x[:-1] count_map[prefix] = count_map.get(prefix, 0) + 1 assert all(e != 1 for e in count_map.values()) + + # Check that the longest hint length isn't too long + if longest_hint_len > min_len and longest_hint_len > 1: + assert num_chars ** (longest_hint_len - 1) < num_elements + + # Check longest hint is not too short + assert num_chars ** longest_hint_len >= num_elements + + if longest_hint_len > min_len and longest_hint_len > 1: + # Check that the longest hint length isn't too long + assert num_chars ** (longest_hint_len - 1) < num_elements + if shortest_hint_len == longest_hint_len: + # Check that we really couldn't use any short links + assert ((num_chars ** longest_hint_len) - num_elements < + len(chars) - 1)