From 0f3aa0bd8c4519411293925416e87feec4404399 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 7 Aug 2015 17:21:18 +0200 Subject: [PATCH 1/8] Ctrl-A only increments number in path segment This prevents a host like "myfoo42.bar" changing to "myfoo43.bar" when pressing Ctrl-A. It further prevents increasing the port number, e.g. going from "foo.bar:8080" to "foo.bar:8081". --- qutebrowser/browser/commands.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index ff117ca38..057b54a24 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -472,9 +472,9 @@ class CommandDispatcher: background: Open the link in a new background tab. window: Open the link in a new window. """ - encoded = bytes(url.toEncoded()).decode('ascii') + path = url.path() # Get the last number in a string - match = re.match(r'(.*\D|^)(\d+)(.*)', encoded) + match = re.match(r'(.*\D|^)(\d+)(.*)', path) if not match: raise cmdexc.CommandError("No number found in URL!") pre, number, post = match.groups() @@ -493,8 +493,10 @@ class CommandDispatcher: val += 1 else: raise ValueError("Invalid value {} for indec!".format(incdec)) - urlstr = ''.join([pre, str(val), post]).encode('ascii') - new_url = QUrl.fromEncoded(urlstr) + new_path = ''.join([pre, str(val), post]) + # Make a copy of the QUrl so we don't modify the original + new_url = QUrl(url) + new_url.setPath(new_path) self._open(new_url, tab, background, window) def _navigate_up(self, url, tab, background, window): From 276b163e0df03cb18a92b656ea43ca02d6ff8981 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 7 Aug 2015 18:48:07 +0200 Subject: [PATCH 2/8] Move logic from _navigate_incdec to urlutils Also add unittests for url_incdec_number --- qutebrowser/browser/commands.py | 28 ++-------------- qutebrowser/utils/urlutils.py | 57 +++++++++++++++++++++++++++++++++ tests/utils/test_urlutils.py | 36 +++++++++++++++++++++ 3 files changed, 96 insertions(+), 25 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 057b54a24..2c9404f93 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -19,7 +19,6 @@ """Command dispatcher for TabbedBrowser.""" -import re import os import shlex import posixpath @@ -472,31 +471,10 @@ class CommandDispatcher: background: Open the link in a new background tab. window: Open the link in a new window. """ - path = url.path() - # Get the last number in a string - match = re.match(r'(.*\D|^)(\d+)(.*)', path) - if not match: - raise cmdexc.CommandError("No number found in URL!") - pre, number, post = match.groups() - if not number: - raise cmdexc.CommandError("No number found in URL!") try: - val = int(number) - except ValueError: - raise cmdexc.CommandError("Could not parse number '{}'.".format( - number)) - if incdec == 'decrement': - if val <= 0: - raise cmdexc.CommandError("Can't decrement {}!".format(val)) - val -= 1 - elif incdec == 'increment': - val += 1 - else: - raise ValueError("Invalid value {} for indec!".format(incdec)) - new_path = ''.join([pre, str(val), post]) - # Make a copy of the QUrl so we don't modify the original - new_url = QUrl(url) - new_url.setPath(new_path) + new_url = urlutils.url_incdec_number(url, incdec) + except urlutils.IncDecError as error: + raise cmdexc.CommandError(error.msg) self._open(new_url, tab, background, window) def _navigate_up(self, url, tab, background, window): diff --git a/qutebrowser/utils/urlutils.py b/qutebrowser/utils/urlutils.py index 50f60d523..f6bfe5701 100644 --- a/qutebrowser/utils/urlutils.py +++ b/qutebrowser/utils/urlutils.py @@ -443,3 +443,60 @@ class FuzzyUrlError(Exception): return self.msg else: return '{}: {}'.format(self.msg, self.url.errorString()) + + +class IncDecError(Exception): + + """Exception raised by url_incdec_number on problems. + + Attributes: + msg: The error message. + url: The QUrl which caused the error. + """ + + def __init__(self, msg, url): + super().__init__(msg) + self.url = url + self.msg = msg + + def __str__(self): + return '{}: {}'.format(self.msg, self.url.toString()) + + +def url_incdec_number(url, incdec): + """Find a number in the url and increment or decrement it. + + Args: + url: The current url + incdec: Either 'increment' or 'decrement' + + Return: + The new url with the number incremented/decremented. + + Raises IncDecError if the url contains no number. + """ + path = url.path() + # Get the last number in a string + match = re.match(r'(.*\D|^)(\d+)(.*)', path) + if not match: + raise IncDecError("No number found in URL!", url) + pre, number, post = match.groups() + if not number: + raise IncDecError("No number found in URL!", url) + try: + val = int(number) + except ValueError: + raise IncDecError("Could not parse number '{}'.".format(number), url) + if incdec == 'decrement': + if val <= 0: + raise IncDecError("Can't decrement {}!".format(val), url) + val -= 1 + elif incdec == 'increment': + val += 1 + else: + raise ValueError("Invalid value {} for indec!".format(incdec)) + new_path = ''.join([pre, str(val), post]) + # Make a copy of the QUrl so we don't modify the original + new_url = QUrl(url) + new_url.setPath(new_path) + return new_url diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index fa1cfa665..520f3d891 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -524,3 +524,39 @@ def test_same_domain_invalid_url(url1, url2): """Test same_domain with invalid URLs.""" with pytest.raises(ValueError): urlutils.same_domain(QUrl(url1), QUrl(url2)) + +@pytest.mark.parametrize('url, incdec, output', [ + ("http://example.com/index1.html", "increment", "http://example.com/index2.html"), + ("http://foo.bar/folder_1/image_2", "increment", "http://foo.bar/folder_1/image_3"), + ("http://bbc.c0.uk:80/story_1", "increment", "http://bbc.c0.uk:80/story_2"), + ("http://mydomain.tld/1_%C3%A4", "increment", "http://mydomain.tld/2_%C3%A4"), + + ("http://example.com/index10.html", "decrement", "http://example.com/index9.html"), + ("http://foo.bar/folder_1/image_3", "decrement", "http://foo.bar/folder_1/image_2"), + ("http://bbc.c0.uk:80/story_1", "decrement", "http://bbc.c0.uk:80/story_0"), + ("http://mydomain.tld/2_%C3%A4", "decrement", "http://mydomain.tld/1_%C3%A4"), +]) +def test_url_incdec_number(url, incdec, output): + """Test url_incdec_number with valid URLs.""" + new_url = urlutils.url_incdec_number(QUrl(url), incdec) + assert new_url == QUrl(output) + +@pytest.mark.parametrize('url', [ + "http://example.com/long/path/but/no/number", + "http://ex4mple.com/number/in/hostname", + "http://example.com:42/number/in/port", + "http://www2.example.com/number/in/subdomain", + "http://example.com/%C3%B6/urlencoded/data", + "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove", +]) +def test_url_incdec_number_invalid(url): + """Test url_incdec_number with URLs that don't contain a number.""" + with pytest.raises(urlutils.IncDecError): + urlutils.url_incdec_number(QUrl(url), "increment") + +def test_url_incdec_number_below_0(): + """Test url_incdec_number with a number that would be below zero + after decrementing.""" + with pytest.raises(urlutils.IncDecError): + urlutils.url_incdec_number(QUrl('http://example.com/page_0.html'), + 'decrement') From c4c3a83ac0c2af1a90a73281cbe123e473c337bf Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Aug 2015 00:08:06 +0200 Subject: [PATCH 3/8] rename url_incdec_number to incdec_number --- qutebrowser/browser/commands.py | 2 +- qutebrowser/utils/urlutils.py | 4 ++-- tests/utils/test_urlutils.py | 18 +++++++++--------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 2c9404f93..ef7a16933 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -472,7 +472,7 @@ class CommandDispatcher: window: Open the link in a new window. """ try: - new_url = urlutils.url_incdec_number(url, incdec) + new_url = urlutils.incdec_number(url, incdec) except urlutils.IncDecError as error: raise cmdexc.CommandError(error.msg) self._open(new_url, tab, background, window) diff --git a/qutebrowser/utils/urlutils.py b/qutebrowser/utils/urlutils.py index f6bfe5701..a1348f41b 100644 --- a/qutebrowser/utils/urlutils.py +++ b/qutebrowser/utils/urlutils.py @@ -447,7 +447,7 @@ class FuzzyUrlError(Exception): class IncDecError(Exception): - """Exception raised by url_incdec_number on problems. + """Exception raised by incdec_number on problems. Attributes: msg: The error message. @@ -463,7 +463,7 @@ class IncDecError(Exception): return '{}: {}'.format(self.msg, self.url.toString()) -def url_incdec_number(url, incdec): +def incdec_number(url, incdec): """Find a number in the url and increment or decrement it. Args: diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index 520f3d891..b6b41eef5 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -536,9 +536,9 @@ def test_same_domain_invalid_url(url1, url2): ("http://bbc.c0.uk:80/story_1", "decrement", "http://bbc.c0.uk:80/story_0"), ("http://mydomain.tld/2_%C3%A4", "decrement", "http://mydomain.tld/1_%C3%A4"), ]) -def test_url_incdec_number(url, incdec, output): - """Test url_incdec_number with valid URLs.""" - new_url = urlutils.url_incdec_number(QUrl(url), incdec) +def test_incdec_number(url, incdec, output): + """Test incdec_number with valid URLs.""" + new_url = urlutils.incdec_number(QUrl(url), incdec) assert new_url == QUrl(output) @pytest.mark.parametrize('url', [ @@ -549,14 +549,14 @@ def test_url_incdec_number(url, incdec, output): "http://example.com/%C3%B6/urlencoded/data", "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove", ]) -def test_url_incdec_number_invalid(url): - """Test url_incdec_number with URLs that don't contain a number.""" +def test_incdec_number_invalid(url): + """Test incdec_number with URLs that don't contain a number.""" with pytest.raises(urlutils.IncDecError): - urlutils.url_incdec_number(QUrl(url), "increment") + urlutils.incdec_number(QUrl(url), "increment") -def test_url_incdec_number_below_0(): - """Test url_incdec_number with a number that would be below zero +def test_incdec_number_below_0(): + """Test incdec_number with a number that would be below zero after decrementing.""" with pytest.raises(urlutils.IncDecError): - urlutils.url_incdec_number(QUrl('http://example.com/page_0.html'), + urlutils.incdec_number(QUrl('http://example.com/page_0.html'), 'decrement') From 9e98ab181a927736b3f74084b71f36363df5b761 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Aug 2015 00:41:17 +0200 Subject: [PATCH 4/8] Add URL validity check + tests to incdec_number --- qutebrowser/utils/urlutils.py | 11 +++++------ tests/utils/test_urlutils.py | 26 +++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/qutebrowser/utils/urlutils.py b/qutebrowser/utils/urlutils.py index a1348f41b..5d8987138 100644 --- a/qutebrowser/utils/urlutils.py +++ b/qutebrowser/utils/urlutils.py @@ -475,18 +475,17 @@ def incdec_number(url, incdec): Raises IncDecError if the url contains no number. """ + if not url.isValid(): + raise ValueError(get_errstring(url)) + path = url.path() # Get the last number in a string match = re.match(r'(.*\D|^)(\d+)(.*)', path) if not match: raise IncDecError("No number found in URL!", url) pre, number, post = match.groups() - if not number: - raise IncDecError("No number found in URL!", url) - try: - val = int(number) - except ValueError: - raise IncDecError("Could not parse number '{}'.".format(number), url) + # This should always succeed because we match \d+ + val = int(number) if incdec == 'decrement': if val <= 0: raise IncDecError("Can't decrement {}!".format(val), url) diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index b6b41eef5..1db7e7a59 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -549,7 +549,7 @@ def test_incdec_number(url, incdec, output): "http://example.com/%C3%B6/urlencoded/data", "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove", ]) -def test_incdec_number_invalid(url): +def test_incdec_number_no_number(url): """Test incdec_number with URLs that don't contain a number.""" with pytest.raises(urlutils.IncDecError): urlutils.incdec_number(QUrl(url), "increment") @@ -560,3 +560,27 @@ def test_incdec_number_below_0(): with pytest.raises(urlutils.IncDecError): urlutils.incdec_number(QUrl('http://example.com/page_0.html'), 'decrement') + +def test_incdec_number_invalid_url(): + """Test if incdec_number rejects an invalid URL.""" + with pytest.raises(ValueError): + urlutils.incdec_number(QUrl(""), "increment") + +def test_incdec_number_wrong_mode(): + """Test if incdec_number rejects a wrong parameter for the incdec + argument.""" + valid_url = QUrl("http://example.com/0") + with pytest.raises(ValueError): + urlutils.incdec_number(valid_url, "foobar") + +@pytest.mark.parametrize("url, msg, expected_str", [ + ("http://example.com", "Invalid", "Invalid: http://example.com"), +]) +def test_incdec_error(url, msg, expected_str): + """Test IncDecError.""" + url = QUrl(url) + with pytest.raises(urlutils.IncDecError) as excinfo: + raise urlutils.IncDecError(msg, url) + + assert excinfo.value.url == url + assert str(excinfo.value) == expected_str From c3f624627413a54f830d8a9847f2d16f37f2fed4 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Aug 2015 00:46:26 +0200 Subject: [PATCH 5/8] Update CHANGELOG.asciidoc --- CHANGELOG.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 08cd555cf..c770fa9cc 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -60,6 +60,8 @@ Fixed - Fixed entering of insert mode when certain disabled text fields were clicked. - Fixed a crash when using `:set` with `-p` and `!` (invert value) - Downloads with unknown size are now handled correctly. +- `:navigate increment/decrement` (``/``) now handles some + corner-cases better. Removed ~~~~~~~ From 72c65a812fafcca0c266367a8e949db49b9c196c Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Aug 2015 13:47:17 +0200 Subject: [PATCH 6/8] Move incdec_number tests to own class and add tests for numbers in anchors --- tests/utils/test_urlutils.py | 111 +++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index 1db7e7a59..d43d806e0 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -525,62 +525,69 @@ def test_same_domain_invalid_url(url1, url2): with pytest.raises(ValueError): urlutils.same_domain(QUrl(url1), QUrl(url2)) -@pytest.mark.parametrize('url, incdec, output', [ - ("http://example.com/index1.html", "increment", "http://example.com/index2.html"), - ("http://foo.bar/folder_1/image_2", "increment", "http://foo.bar/folder_1/image_3"), - ("http://bbc.c0.uk:80/story_1", "increment", "http://bbc.c0.uk:80/story_2"), - ("http://mydomain.tld/1_%C3%A4", "increment", "http://mydomain.tld/2_%C3%A4"), +class TestIncDecNumber: - ("http://example.com/index10.html", "decrement", "http://example.com/index9.html"), - ("http://foo.bar/folder_1/image_3", "decrement", "http://foo.bar/folder_1/image_2"), - ("http://bbc.c0.uk:80/story_1", "decrement", "http://bbc.c0.uk:80/story_0"), - ("http://mydomain.tld/2_%C3%A4", "decrement", "http://mydomain.tld/1_%C3%A4"), -]) -def test_incdec_number(url, incdec, output): - """Test incdec_number with valid URLs.""" - new_url = urlutils.incdec_number(QUrl(url), incdec) - assert new_url == QUrl(output) + """Tests for urlutils.incdec_number().""" -@pytest.mark.parametrize('url', [ - "http://example.com/long/path/but/no/number", - "http://ex4mple.com/number/in/hostname", - "http://example.com:42/number/in/port", - "http://www2.example.com/number/in/subdomain", - "http://example.com/%C3%B6/urlencoded/data", - "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove", -]) -def test_incdec_number_no_number(url): - """Test incdec_number with URLs that don't contain a number.""" - with pytest.raises(urlutils.IncDecError): - urlutils.incdec_number(QUrl(url), "increment") + @pytest.mark.parametrize('url, incdec, output', [ + ("http://example.com/index1.html", "increment", "http://example.com/index2.html"), + ("http://foo.bar/folder_1/image_2", "increment", "http://foo.bar/folder_1/image_3"), + ("http://bbc.c0.uk:80/story_1", "increment", "http://bbc.c0.uk:80/story_2"), + ("http://mydomain.tld/1_%C3%A4", "increment", "http://mydomain.tld/2_%C3%A4"), + ("http://example.com/site/5#5", "increment", "http://example.com/site/6#5"), -def test_incdec_number_below_0(): - """Test incdec_number with a number that would be below zero - after decrementing.""" - with pytest.raises(urlutils.IncDecError): - urlutils.incdec_number(QUrl('http://example.com/page_0.html'), - 'decrement') + ("http://example.com/index10.html", "decrement", "http://example.com/index9.html"), + ("http://foo.bar/folder_1/image_3", "decrement", "http://foo.bar/folder_1/image_2"), + ("http://bbc.c0.uk:80/story_1", "decrement", "http://bbc.c0.uk:80/story_0"), + ("http://mydomain.tld/2_%C3%A4", "decrement", "http://mydomain.tld/1_%C3%A4"), + ("http://example.com/site/5#5", "decrement", "http://example.com/site/4#5"), + ]) + def test_incdec_number(self, url, incdec, output): + """Test incdec_number with valid URLs.""" + new_url = urlutils.incdec_number(QUrl(url), incdec) + assert new_url == QUrl(output) -def test_incdec_number_invalid_url(): - """Test if incdec_number rejects an invalid URL.""" - with pytest.raises(ValueError): - urlutils.incdec_number(QUrl(""), "increment") + @pytest.mark.parametrize('url', [ + "http://example.com/long/path/but/no/number", + "http://ex4mple.com/number/in/hostname", + "http://example.com:42/number/in/port", + "http://www2.example.com/number/in/subdomain", + "http://example.com/%C3%B6/urlencoded/data", + "http://example.com/number/in/anchor#5", + "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove#5", + ]) + def test_no_number(self, url): + """Test incdec_number with URLs that don't contain a number.""" + with pytest.raises(urlutils.IncDecError): + urlutils.incdec_number(QUrl(url), "increment") -def test_incdec_number_wrong_mode(): - """Test if incdec_number rejects a wrong parameter for the incdec - argument.""" - valid_url = QUrl("http://example.com/0") - with pytest.raises(ValueError): - urlutils.incdec_number(valid_url, "foobar") + def test_number_below_0(self): + """Test incdec_number with a number that would be below zero + after decrementing.""" + with pytest.raises(urlutils.IncDecError): + urlutils.incdec_number(QUrl('http://example.com/page_0.html'), + 'decrement') -@pytest.mark.parametrize("url, msg, expected_str", [ - ("http://example.com", "Invalid", "Invalid: http://example.com"), -]) -def test_incdec_error(url, msg, expected_str): - """Test IncDecError.""" - url = QUrl(url) - with pytest.raises(urlutils.IncDecError) as excinfo: - raise urlutils.IncDecError(msg, url) + def test_invalid_url(self): + """Test if incdec_number rejects an invalid URL.""" + with pytest.raises(ValueError): + urlutils.incdec_number(QUrl(""), "increment") - assert excinfo.value.url == url - assert str(excinfo.value) == expected_str + def test_wrong_mode(self): + """Test if incdec_number rejects a wrong parameter for the incdec + argument.""" + valid_url = QUrl("http://example.com/0") + with pytest.raises(ValueError): + urlutils.incdec_number(valid_url, "foobar") + + @pytest.mark.parametrize("url, msg, expected_str", [ + ("http://example.com", "Invalid", "Invalid: http://example.com"), + ]) + def test_incdec_error(self, url, msg, expected_str): + """Test IncDecError.""" + url = QUrl(url) + with pytest.raises(urlutils.IncDecError) as excinfo: + raise urlutils.IncDecError(msg, url) + + assert excinfo.value.url == url + assert str(excinfo.value) == expected_str From bb6d6e51f67cd313fa9af05a2b407a4ba7489915 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Aug 2015 13:59:43 +0200 Subject: [PATCH 7/8] Remove trailing whitespace in test_urlutils.py --- tests/utils/test_urlutils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index d43d806e0..cb07ca80e 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -534,13 +534,13 @@ class TestIncDecNumber: ("http://foo.bar/folder_1/image_2", "increment", "http://foo.bar/folder_1/image_3"), ("http://bbc.c0.uk:80/story_1", "increment", "http://bbc.c0.uk:80/story_2"), ("http://mydomain.tld/1_%C3%A4", "increment", "http://mydomain.tld/2_%C3%A4"), - ("http://example.com/site/5#5", "increment", "http://example.com/site/6#5"), + ("http://example.com/site/5#5", "increment", "http://example.com/site/6#5"), ("http://example.com/index10.html", "decrement", "http://example.com/index9.html"), ("http://foo.bar/folder_1/image_3", "decrement", "http://foo.bar/folder_1/image_2"), ("http://bbc.c0.uk:80/story_1", "decrement", "http://bbc.c0.uk:80/story_0"), ("http://mydomain.tld/2_%C3%A4", "decrement", "http://mydomain.tld/1_%C3%A4"), - ("http://example.com/site/5#5", "decrement", "http://example.com/site/4#5"), + ("http://example.com/site/5#5", "decrement", "http://example.com/site/4#5"), ]) def test_incdec_number(self, url, incdec, output): """Test incdec_number with valid URLs.""" From cef0ac46b8eed7c92d4c0ebcc7fff85fdaceaea3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 8 Aug 2015 15:16:18 +0200 Subject: [PATCH 8/8] Update authors. --- README.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/README.asciidoc b/README.asciidoc index e2fd39a13..4b072684c 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -146,6 +146,7 @@ Contributors, sorted by the number of commits in descending order: * ZDarian * Peter Vilim * John ShaggyTwoDope Jenkins +* Daniel * Jimmy * Alexander Cogneau * Zach-Button