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

add 'allow_html_redirect' configuration option to avoid printing warning when redirecting .html URLs #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boegel
Copy link

@boegel boegel commented Oct 23, 2022

I'm porting documentation that currently uses Sphinx + .rst to MkDocs + md, and I would like to ensure that redirects are in place for old URLs like https://docs.easybuild.io/en/latest/Configuration.html.

To achieve this, I'm using mkdocs-redirects, as follows in mkdocs.yml:

plugins:
  - redirects:
      redirect_maps:
        en/latest/Configuration.html: configuring_easybuild.md

That's working fine (see https://easybuilders.github.io/easybuild-docs/en/latest/Configuration.html which correctly redirects), but mkdocs-redirects is logging warnings when .html URLs are redirected:

WARNING  -  redirects plugin: 'en/latest/Configuration.html' is not a valid markdown file!

That's annoying, since I would like to use mkdocs build --strict in CI to test changes to our documentation, and that exits with a non-zero exit code as soon there as any warnings.

The changes being proposed here allow configuring mkdocs-redirects with allow_html_redirects to silence the warning:

plugins:
  - redirects:
      allow_html_redirect: True
      redirect_maps:
        en/latest/Configuration.html: configuring_easybuild.md

@boegel
Copy link
Author

boegel commented Oct 23, 2022

I'm happy to make the extra effort in this PR to update the README file with a new section that documents this feature, and to implement a test to verify that the feature works as expected, but I would like to get some feedback first on whether this feature would be accepted for inclusing in mkdocs-redirects before doing so.

@oprypin
Copy link
Contributor

oprypin commented Oct 23, 2022

Just a few days ago I was thinking that something like this would be good, because indeed, the current setup assumes you're migrating from an exact same MkDocs setup (even down to use_directory_urls). And I actually think this shouldn't even be behind a config. But I'll need to carefully look at the code myself first. It would be good to have this as an issue instead, to clearly define what use case we're solving. Though I see that in the description of your pull request you do define the use case very well, definitely thanks for that!

@boegel
Copy link
Author

boegel commented Oct 24, 2022

@oprypin Thanks for the feedback, I've opened an issue for this at #51

@jrappen
Copy link

jrappen commented Jan 26, 2023

@oprypin what's the current status here?

# Validate user-provided redirect "old files"
for page_old in self.redirects.keys():
if not utils.is_markdown_file(page_old):
log.warning("redirects plugin: '%s' is not a valid markdown file!", page_old)
if not (allow_html and page_old.lower().endswith(('html', 'htm'))):
log.warning("redirects plugin: '%s' is not a valid markdown file!", page_old)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ... not a valid md or html ..., yes?

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

Successfully merging this pull request may close these issues.

None yet

3 participants