From 3bfafb5e5066b40dbe41cffe1e0fc3d27e504cfd Mon Sep 17 00:00:00 2001
From: Iordanis Grigoriou <iordanis.grigoriou@gmail.com>
Date: Thu, 6 Jul 2017 17:41:54 +0200
Subject: [PATCH] Refactor suggested_fn_from_title, add unit tests

---
 qutebrowser/browser/commands.py             | 11 +++-------
 qutebrowser/browser/downloads.py            | 20 +++++++++++++++++
 tests/end2end/features/downloads.feature    |  7 ------
 tests/unit/browser/webkit/test_downloads.py | 24 +++++++++++++++++++++
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index 88499c412..3e918a32a 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1444,14 +1444,9 @@ class CommandDispatcher:
         else:
             qnam = tab.networkaccessmanager()
 
-            # Downloads of URLs with file extensions in the whitelist will use
-            # the page title as the filename.
-            ext_whitelist = [".html", ".htm", ".php", ""]
-            _, ext = os.path.splitext(self._current_url().path())
-            if ext.lower() in ext_whitelist and tab.title():
-                suggested_fn = utils.sanitize_filename(tab.title()) + ext
-            else:
-                suggested_fn = None
+            suggested_fn = downloads.suggested_fn_from_title(
+                self._current_url().path(), tab.title()
+            )
 
             download_manager.get(
                 self._current_url(),
diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index c7cb5ad8e..35b3d26c6 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -182,6 +182,26 @@ def transform_path(path):
     return path
 
 
+def suggested_fn_from_title(url, title=None):
+    """Suggest a filename depending on the URL extension and page title.
+    Args:
+        url: a string with the URL path
+        title: the page title string
+
+    Returns None if the extension is not in the whitelist
+    or if there is no page title.
+    """
+    ext_whitelist = [".html", ".htm", ".php", ""]
+    _, ext = os.path.splitext(url)
+    if ext.lower() in ext_whitelist and title:
+        suggested_fn = utils.sanitize_filename(title)
+        if not suggested_fn.endswith(ext):
+            suggested_fn += ext
+    else:
+        suggested_fn = None
+    return suggested_fn
+
+
 class NoFilenameError(Exception):
 
     """Raised when we can't find out a filename in DownloadTarget."""
diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature
index 40333aee6..f636b3247 100644
--- a/tests/end2end/features/downloads.feature
+++ b/tests/end2end/features/downloads.feature
@@ -36,13 +36,6 @@ Feature: Downloading things from a website.
         And I wait until the download is finished
         Then the downloaded file qutebrowser.png should exist
 
-    Scenario: Using :download with no URL and no page title
-        When I set storage -> prompt-download-directory to false
-        And I open data/downloads/download with no title.html
-        And I run :download
-        And I wait until the download is finished
-        Then the downloaded file download with no title.html should exist
-
     Scenario: Using hints
         When I set storage -> prompt-download-directory to false
         And I open data/downloads/downloads.html
diff --git a/tests/unit/browser/webkit/test_downloads.py b/tests/unit/browser/webkit/test_downloads.py
index 72b533cc7..d22338717 100644
--- a/tests/unit/browser/webkit/test_downloads.py
+++ b/tests/unit/browser/webkit/test_downloads.py
@@ -30,6 +30,30 @@ def test_download_model(qapp, qtmodeltester, config_stub, cookiejar_and_cache):
     qtmodeltester.check(model)
 
 
+@pytest.mark.parametrize('url, title, out', [
+    ('https://qutebrowser.org/img/cheatsheet-big.png',
+     'cheatsheet-big.png (3342×2060)',
+     None),
+    ('http://qutebrowser.org/INSTALL.html',
+     'Installing qutebrowser | qutebrowser',
+     'Installing qutebrowser _ qutebrowser.html'),
+    ('http://qutebrowser.org/INSTALL.html.html',
+     'Installing qutebrowser | qutebrowser',
+     'Installing qutebrowser _ qutebrowser.html'),
+    ('http://qutebrowser.org/',
+     'qutebrowser | qutebrowser',
+     'qutebrowser _ qutebrowser'),
+    ('https://github.com/qutebrowser/qutebrowser/releases',
+     'Releases · qutebrowser/qutebrowser',
+     'Releases · qutebrowser_qutebrowser'),
+    ('http://qutebrowser.org/page-with-no-title.html',
+     '',
+     None),
+])
+def test_page_titles(url, title, out):
+    assert downloads.suggested_fn_from_title(url, title) == out
+
+
 class TestDownloadTarget:
 
     def test_base(self):