Merge pull request #5349 from tk0miya/5348_support_remote_download_file

Fix #5348: download reference to remote file is not displayed

CHANGES

@@ -24,6 +24,7 @@ Bugs fixed
 * #5325: latex: cross references has been broken by multiply labeled objects
 * C++, fixes for symbol addition and lookup. Lookup should no longer break
   in partial builds. See also #5337.
+* #5348: download reference to remote file is not displayed
 
 Testing
 --------

sphinx/environment/collectors/asset.py

@@ -128,13 +128,16 @@ class DownloadFileCollector(EnvironmentCollector):
         """Process downloadable file paths. """
         for node in doctree.traverse(addnodes.download_reference):
             targetname = node['reftarget']
-            rel_filename, filename = app.env.relfn2path(targetname, app.env.docname)
+            if '://' in targetname:
-            app.env.dependencies[app.env.docname].add(rel_filename)
+                node['refuri'] = targetname
-            if not os.access(filename, os.R_OK):
+            else:
-                logger.warning(__('download file not readable: %s') % filename,
+                rel_filename, filename = app.env.relfn2path(targetname, app.env.docname)
-                               location=node, type='download', subtype='not_readable')
+                app.env.dependencies[app.env.docname].add(rel_filename)
-                continue
+                if not os.access(filename, os.R_OK):
-            node['filename'] = app.env.dlfiles.add_file(app.env.docname, filename)
+                    logger.warning(__('download file not readable: %s') % filename,
+                                   location=node, type='download', subtype='not_readable')
+                    continue
+                node['filename'] = app.env.dlfiles.add_file(app.env.docname, filename)
 
 
 def setup(app):

sphinx/writers/html.py

@@ -564,10 +564,20 @@ class HTMLTranslator(BaseTranslator):
 
     def visit_download_reference(self, node):
         # type: (nodes.Node) -> None
-        if self.builder.download_support and node.hasattr('filename'):
+        atts = {'class': 'reference download',
-            self.body.append(
+                'download': ''}
-                '<a class="reference download internal" href="%s" download="">' %
+
-                posixpath.join(self.builder.dlpath, node['filename']))
+        if not self.builder.download_support:
+            self.context.append('')
+        elif 'refuri' in node:
+            atts['class'] += ' external'
+            atts['href'] = node['refuri']
+            self.body.append(self.starttag(node, 'a', '', **atts))
+            self.context.append('</a>')
+        elif 'filename' in node:
+            atts['class'] += ' internal'
+            atts['href'] = posixpath.join(self.builder.dlpath, node['filename'])
+            self.body.append(self.starttag(node, 'a', '', **atts))
             self.context.append('</a>')
         else:
             self.context.append('')

sphinx/writers/html5.py

@@ -510,10 +510,20 @@ class HTML5Translator(BaseTranslator):
 
     def visit_download_reference(self, node):
         # type: (nodes.Node) -> None
-        if self.builder.download_support and node.hasattr('filename'):
+        atts = {'class': 'reference download',
-            self.body.append(
+                'download': ''}
-                '<a class="reference download internal" href="%s" download="">' %
+
-                posixpath.join(self.builder.dlpath, node['filename']))
+        if not self.builder.download_support:
+            self.context.append('')
+        elif 'refuri' in node:
+            atts['class'] += ' external'
+            atts['href'] = node['refuri']
+            self.body.append(self.starttag(node, 'a', '', **atts))
+            self.context.append('</a>')
+        elif 'filename' in node:
+            atts['class'] += ' internal'
+            atts['href'] = posixpath.join(self.builder.dlpath, node['filename'])
+            self.body.append(self.starttag(node, 'a', '', **atts))
             self.context.append('</a>')
         else:
             self.context.append('')

tests/roots/test-roles-download/conf.py

@@ -0,0 +1,7 @@
+# -*- coding: utf-8 -*-
+
+master_doc = 'index'
+
+latex_documents = [
+    (master_doc, 'test.tex', 'The basic Sphinx documentation for testing', 'Sphinx', 'report')
+]

tests/roots/test-roles-download/index.rst

@@ -0,0 +1,6 @@
+test-roles-download
+===================
+
+* :download:`dummy.dat`
+* :download:`not_found.dat`
+* :download:`Sphinx logo <http://www.sphinx-doc.org/en/master/_static/sphinxheader.png>`

tests/test_build_epub.py

@@ -354,6 +354,22 @@ def test_epub_css_files(app):
             'href="https://example.com/custom.css" />' not in content)
 
 
+@pytest.mark.sphinx('epub', testroot='roles-download')
+def test_html_download_role(app, status, warning):
+    app.build()
+    assert not (app.outdir / '_downloads' / 'dummy.dat').exists()
+
+    content = (app.outdir / 'index.xhtml').text()
+    assert ('<li><p><code class="xref download docutils literal notranslate">'
+            '<span class="pre">dummy.dat</span></code></p></li>' in content)
+    assert ('<li><p><code class="xref download docutils literal notranslate">'
+            '<span class="pre">not_found.dat</span></code></p></li>' in content)
+    assert ('<li><p><code class="xref download docutils literal notranslate">'
+            '<span class="pre">Sphinx</span> <span class="pre">logo</span></code>'
+            '<span class="link-target"> [http://www.sphinx-doc.org/en/master'
+            '/_static/sphinxheader.png]</span></p></li>' in content)
+
+
 @pytest.mark.sphinx('epub')
 def test_run_epubcheck(app):
     app.build()

tests/test_build_html.py

@@ -1387,3 +1387,22 @@ def test_html_math_renderer_is_mismatched(make_app, app_params):
         assert False
     except ConfigError as exc:
         assert str(exc) == "Unknown math_renderer 'imgmath' is given."
+
+
+@pytest.mark.sphinx('html', testroot='roles-download')
+def test_html_download_role(app, status, warning):
+    app.build()
+    assert (app.outdir / '_downloads' / 'dummy.dat').exists()
+
+    content = (app.outdir / 'index.html').text()
+    assert ('<li><a class="reference download internal" download="" '
+            'href="_downloads/dummy.dat">'
+            '<code class="xref download docutils literal notranslate">'
+            '<span class="pre">dummy.dat</span></code></a></li>' in content)
+    assert ('<li><code class="xref download docutils literal notranslate">'
+            '<span class="pre">not_found.dat</span></code></li>' in content)
+    assert ('<li><a class="reference download external" download="" '
+            'href="http://www.sphinx-doc.org/en/master/_static/sphinxheader.png">'
+            '<code class="xref download docutils literal notranslate">'
+            '<span class="pre">Sphinx</span> <span class="pre">logo</span>'
+            '</code></a></li>' in content)

tests/test_build_html5.py

@@ -321,3 +321,23 @@ def test_html5_output(app, cached_etree_parse, fname, expect):
     app.build()
     print(app.outdir / fname)
     check_xpath(cached_etree_parse(app.outdir / fname), fname, *expect)
+
+
+@pytest.mark.sphinx('html', testroot='roles-download',
+                    confoverrides={'html_experimental_html5_writer': True})
+def test_html_download_role(app, status, warning):
+    app.build()
+    assert (app.outdir / '_downloads' / 'dummy.dat').exists()
+
+    content = (app.outdir / 'index.html').text()
+    assert ('<li><p><a class="reference download internal" download="" '
+            'href="_downloads/dummy.dat">'
+            '<code class="xref download docutils literal notranslate">'
+            '<span class="pre">dummy.dat</span></code></a></p></li>' in content)
+    assert ('<li><p><code class="xref download docutils literal notranslate">'
+            '<span class="pre">not_found.dat</span></code></p></li>' in content)
+    assert ('<li><p><a class="reference download external" download="" '
+            'href="http://www.sphinx-doc.org/en/master/_static/sphinxheader.png">'
+            '<code class="xref download docutils literal notranslate">'
+            '<span class="pre">Sphinx</span> <span class="pre">logo</span>'
+            '</code></a></p></li>' in content)