Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing PDF icons in the Sage documentation website #30418

Closed
kwankyu opened this issue Aug 22, 2020 · 17 comments
Closed

Missing PDF icons in the Sage documentation website #30418

kwankyu opened this issue Aug 22, 2020 · 17 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 22, 2020

This ticket is a spin-off from #29993.

In local/share/doc/sage/html/en/index.html, there are supposed to be PDF icons next to each document, as links leading to the PDF builds. Those are missing, because the file _static/pdf.png is missing. For some reason, it is not copied from src/doc/en/website/static to local/share/doc/sage/html/en/_static.

To see the problem, check that:

Running make doc-clean; sage --docbuild all html does not install the PDF icon to the right place, that is under local/share/doc/sage/html/en/website/_static.

But running make doc-clean; sage --docbuild website html does install the PDF icon to the right place.

CC: @jhpalmieri

Component: documentation

Author: Kwankyu Lee

Branch/Commit: f69c403

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/30418

@kwankyu kwankyu added this to the sage-9.2 milestone Aug 22, 2020
@kwankyu

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

This change works:

diff --git a/src/sage_setup/docbuild/__init__.py b/src/sage_setup/docbuild/__init__.py
index 0841f429e7..9c2e6ed4e2 100644
--- a/src/sage_setup/docbuild/__init__.py
+++ b/src/sage_setup/docbuild/__init__.py
@@ -580,9 +580,6 @@ class ReferenceBuilder(AllBuilder):
             # of the PDF file.  So we create an html file, based on
             # the file index.html from the "website" target.
             if format == 'pdf':
-                # First build the website page. This only takes a few seconds.
-                getattr(get_builder('website'), 'html')()
-
                 website_dir = os.path.join(SAGE_DOC, 'html', 'en', 'website')
                 output_dir = self._output_dir(format, lang)
 
@@ -1726,5 +1723,8 @@ def main():
     # Set up Intersphinx cache
     C = IntersphinxCache()
 
+    if type == 'pdf':
+        # First build the website page. This only takes a few seconds.
+        getattr(get_builder('website'), 'html')()
     builder = getattr(get_builder(name), type)
     builder()

This moves the test to the top-level main. We could certainly test for more than if type == 'pdf, for example also name == 'all' or name == 'reference'. Or maybe it's just a good idea to always build the website html page and get the pdf icon there.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 7, 2020

comment:4

Replying to @jhpalmieri:

The code in lines 583--585 has nothing to do with the current issue. It should not be removed.

Your code in lines 1726--1728 is effectively the same with running sage --docbuild website html. This just covers the issue.

The issue of this ticket is to see why sage --docbuild all html does not install the pdf icon. The pdf icon is not installed because the website builder does not copy its static files folder. I checked this problem is also with the developer manual builder.

@jhpalmieri
Copy link
Member

comment:5

Replying to @kwankyu:

Replying to @jhpalmieri:

The code in lines 583--585 has nothing to do with the current issue.

I think that code is intended to build the basic website framework for the PDF documentation. It appears to matter when getattr(get_builder('website'), 'html')() is executed, as far as installing the appropriate static files. Can you explain why?

It should not be removed.

Of course I didn't remove it, I moved it.

Your code in lines 1726--1728 is effectively the same with running sage --docbuild website html. This just covers the issue.

I know that. The question is why it works and why running getattr(get_builder('website'), 'html')() at a different stage does not.

The issue of this ticket is to see why sage --docbuild all html does not install the pdf icon.

I disagree: you don't want the PDF icon if you haven't built the PDF documentation. So sage --docbuild all html should ideally not install the PDF icon.

The pdf icon is not installed because the website builder does not copy its static files folder. I checked this problem is also with the developer manual builder.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 7, 2020

comment:6

Replying to @jhpalmieri:

Replying to @kwankyu:

Replying to @jhpalmieri:

The code in lines 583--585 has nothing to do with the current issue.

I think that code is intended to build the basic website framework for the PDF documentation. It appears to matter when getattr(get_builder('website'), 'html')() is executed, as far as installing the appropriate static files. Can you explain why?

The main purpose of the code is to provide the htmls for the basic website framework. The other code following it assumes that the htmls are in place. If you remove the code from the place where it is, the assumption is broken.

Your code in lines 1726--1728 is effectively the same with running sage --docbuild website html. This just covers the issue.

I know that. The question is why it works and why running getattr(get_builder('website'), 'html')() at a different stage does not.

The question must be solved before we think of any cure.

The issue of this ticket is to see why sage --docbuild all html does not install the pdf icon.

I disagree: you don't want the PDF icon if you haven't built the PDF documentation. So sage --docbuild all html should ideally not install the PDF icon.

The PDF icon problem is just a consequence of the bigger problem of sage --docbuild all html not copying the static files (of website and developer documentations.)

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 12, 2020

Branch: u/klee/30418

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 12, 2020

Author: Kwankyu Lee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2020

Commit: 245a1a3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

245a1a3Add html_common_static_path to fix a bug in conf.py

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

28b0224Remove u prefix from strings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

Changed commit from 245a1a3 to 28b0224

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

Changed commit from 28b0224 to f69c403

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f69c403Remove u prefix from strings

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 29, 2020

comment:12

It would be nice for this ticket to get reviewed for sage 9.2.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:13

Looks okay to me.

@vbraun
Copy link
Member

vbraun commented Nov 7, 2020

Changed branch from u/klee/30418 to f69c403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants