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 index.html generation for redirecting to ./stable or ./dev #1657

Merged
merged 15 commits into from
Aug 1, 2021

Conversation

hyrodium
Copy link
Contributor

This PR solves #937.

@mortenpi
Copy link
Member

Thanks @hyrodium, this is great!

I am concerned that this will overwrite user-created files -- there might be some cases where the user wants to maintain their own index.html. So if we want this in a patch release, we should not touch an existing index.html.

However, the URL might change (e.g. dev/ -> v1/ -> v2/). So it is probably good if Documenter updates the file when necessary. I see two options:

  1. We could potentially just change the behavior in the next breaking release to always update index.html.
  2. Check for an HTML comment in index.html that indicates that the current version is automatically generated, and so can be overwritten.

I am kinda partial two (2), even if it's a little more complicated, since that still allows the user to also manually maintain an index.html file.

@mortenpi mortenpi added this to the 0.27.5 milestone Jul 24, 2021
@hyrodium
Copy link
Contributor Author

Thanks for the review!
I've updated the function to check the existence of a comment "<!--This file is automatically generated by Documenter.jl-->" in index.html.

src/Writers/HTMLWriter.jl Outdated Show resolved Hide resolved
@hyrodium hyrodium mentioned this pull request Jul 24, 2021
@fredrikekre
Copy link
Member

Check for an HTML comment in index.html that indicates that the current version is automatically generated, and so can be overwritten.

This was what I was thinking too, and looks like you implemented that now in the PR so 👍

@hyrodium
Copy link
Contributor Author

I've fixed the redirected destination, and I think this is ready to merge.

Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

LGTM, can you add an entry to the NEWS.md file?

@hyrodium
Copy link
Contributor Author

I've updated the CHANGELOG.md. (btw, not NEWS.md:smile:)

Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

.

CHANGELOG.md Outdated Show resolved Hide resolved
@fredrikekre
Copy link
Member

LGTM, thanks. I'll let @mortenpi look over it and merge.

@mortenpi mortenpi removed this from the 0.27.5 milestone Aug 1, 2021
@mortenpi mortenpi added this to the 0.27.6 milestone Aug 1, 2021
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hyrodium and @fredrikekre!

CHANGELOG.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants