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

Breaking change in 0.2.7 - Double .html filename extension showing up #46

Closed
consideRatio opened this issue Apr 16, 2021 · 7 comments
Closed

Comments

@consideRatio
Copy link

Thanks everyone involved for making this lovely extension - it is great ❤️

#45 seem to have broken the documentation setup in this project using myst-parser along with sphinxext-rediraffe. I'm not sure if it is to be considered a bug in this project or somewhere else how we are using it end to end but I figured it could be good to quickly report the experience here.

I can go back and forth between the documentation build failure by installing 0.2.7 and 0.2.6 without changing anything else. I'm using the command make html to reproduce the error, which is in practice sphinx build in some way.

# 0.2.6 works like this:
The HTML pages are in build/html.
Writing redirects...
(good) customizing/user-management.html --> jupyterhub/customizing/user-management.html
(good) customizing/user-storage.html --> jupyterhub/customizing/user-storage.html

# 0.2.7 works like this:
The HTML pages are in build/html.
Writing redirects...
WARNING: (broken) customizing/user-management.html.html redirects to jupyterhub/customizing/user-management.html.html but /home/erik/dev/contrib/jupyterhub/zero-to-jupyterhub-k8s/doc/build/html/jupyterhub/customizing/user-management.html.html does not exist!
WARNING: (broken) customizing/user-storage.html.html redirects to jupyterhub/customizing/user-storage.html.html but /home/erik/dev/contrib/jupyterhub/zero-to-jupyterhub-k8s/doc/build/html/jupyterhub/customizing/user-storage.html.html does not exist!
@consideRatio consideRatio changed the title Breaking change in 0.2.7 Breaking change in 0.2.7 - Double .html extension showing up Apr 16, 2021
@consideRatio consideRatio changed the title Breaking change in 0.2.7 - Double .html extension showing up Breaking change in 0.2.7 - Double .html filename extension showing up Apr 16, 2021
@consideRatio
Copy link
Author

Ping @chrisjsewell as you are very familiar with myst-parser and the author of #45 which seems related

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Apr 16, 2021

Ha yeh well I'll leterally made the PR to fix a project where I am also using myst-parser, but its actually not to do with that.

It is because you have listed your redirect files with .html extensions (https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/c819835794a5a42f7a5016c0f3cdd952c28d48a3/doc/source/conf.py#L141), e.g.:

 "customizing/user-management.html": "jupyterhub/customizing/user-management.html",

which as of #45 will no longer be replaced, only suffixes related with a source document (like .rst or .md).
Is there a reason you are adding the .html extensions to the redirects? I'm not sure thats how rediraffe is intended to write these redirects (obviously @TheTripleV et al can clarify). If you simply remove them then it will work, i.e.:

 "customizing/user-management": "jupyterhub/customizing/user-management",

or

 "customizing/user-management.md": "jupyterhub/customizing/user-management.md",

I notice in one of your redirects also you use resources/community.html#links-to-community-project-resources, which I also don't think rediraffe has the capability to do currently, i.e. it was previously just converting that to resources/community then adding .html

@chrisjsewell
Copy link
Contributor

if we didn't want to recover this behaviour, then you could add some extra code to:

def remove_suffix(docname: str, suffixes: List[str]) -> str:

to also strip .html and potentially .html#...

@TheTripleV
Copy link
Member

TheTripleV commented Apr 16, 2021

1. Regarding redirecting to urls with fragments and queries:

rediraffe passes fragments and queries from the original url to the new url.
Ex: If a.html is redirected to b.html, then visiting a.html#abc will go to b.html#abc.
I now realize that this is not documented. I don't think specifying fragments or queries in the rediraffe config works. Iirc, the problem I faced in the original design was how to handle internal urls with fragments.
If the config is

a b#f1
b c

, then should the fragment f1 be propagated through or disappear? Should users be allowed to specify a separate redirect b#f1 d for individual fragments?

2. Regarding the core issue, doubling html extensions

This extension was intended to have the source suffix specified in the redirects config.
(a.rst b.rst instead of a b or a.html b.html)
While the 2 alternatives seem to work for the core functionality of creating redirects, I don't believe it works with our diff checking builder that makes sure links don't break. This is actually why our tests didn't catch this; All of our test cases include the source suffix in their configs.
I'd rather enforce that the source suffix is included in the rediraffe config. But, I am open to a good reason why destination suffixes or no suffixes should be allowed.

@TheTripleV
Copy link
Member

If point 1 is something you'd like implemented, please do open an issue.

@consideRatio
Copy link
Author

Thanks for this very helpful discussion, I've made my build succeed by updating a.html b.html to a b and removed any #anchors suffixing b.

I'd rather enforce that the source suffix is included in the rediraffe config. But, I am open to a good reason why destination suffixes or no suffixes should be allowed.

Hmmmm I deliberated a bit about this without so good project knowledge. But I was thinking that if you have a a.rst b.rst redirection rule because a.rst was renamed b.rst, then you no longer have the file a.rst and could in practice not really enforce such file existed or had .rst or .md extension etc?

Then I also got thinking, if one would have b.rst c.rst because b.rst was later renamed to c.rst, one wouldn't have a file representing a.rst or b.rst that was part of the original redirection rule a.rst b.rst which still serves a purpose in a a->b->c redirection sequence.


I'll go ahead and close this issue, thank you for the assistance to migrate my projects configuration and for an excellent sphinx extension!

@consideRatio
Copy link
Author

Is there a reason you are adding the .html extensions to the redirects?

Open source :) It wasn't me who set this up originally so I don't know.

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

No branches or pull requests

3 participants