Merge remote-tracking branch 'origin/pr/4330'

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
+            # For each 1 short link being added, len(chars) long links are
-            # dividing by the number of spots we would loose by adding a
+            # removed, therefore the space removed is len(chars) - 1.
-            # short element
+            short_count = (total_space - len(elems)) // (len(chars) - 1)
-            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
 
@@ -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

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

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_chars', [5, 9])
-@pytest.mark.parametrize('num_elements', range(1, 26))
+@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)