Force index file creation on mkdocs #3635
Conversation
|
Looks like a good PR. This is something we should figure out how to test well, and include that here. I realize it might be a rabbit hole to figure out testing, but it feels hard to really do a lot of changes here without something to test it. |
|
@ericholscher I have added some tests, not sure if they are on the correct place. |
| python_env = Virtualenv(version=version, build_env=build_env) | ||
| base_builder = MkdocsHTML(build_env, python_env) | ||
|
|
||
| def look_index_path(path): |
There was a problem hiding this comment.
I know there is a function to fake paths based on a regex, but with this form looks more explicit, at least for me.
| return True | ||
| elif path.endswith('index.md'): | ||
| return False | ||
| return False |
There was a problem hiding this comment.
This is repeated 4 times, I think it should be a util method at the top of the file?
There was a problem hiding this comment.
I will add this method to the mock path utils better :)
readthedocs/doc_builder/base.py
Outdated
|
|
||
| Welcome to Read the Docs | ||
| ------------------------ | ||
|
|
||
| This is an autogenerated index file. | ||
|
|
||
| Please create an ``index.{ext}`` or ``README.{ext}`` file with your own content | ||
| under the root (or ``/docs``) directory in your repository. | ||
| Please create an ``index.{ext}`` or ``README.{ext}`` (Sphinx only) file with |
There was a problem hiding this comment.
Why is this logic Sphinx only? Do we actually do it for Sphinx projects, but not mkdocs? Should we do it for Sphinx projects?
There was a problem hiding this comment.
I was thinking the same, I'm going to check if this works on Sphinx, if so, probably force_index would be the default behavior.
|
@ericholscher I take a look at the code, and notice that the template wasn't using the I did a test using the Also notice that now with just a |
I just noticed the same in locally and I wanted to report this but it seems that you already knew it. My manual test was a simple repository with just |
|
I labeling this as |
stsewd
force-pushed
the
force-index-file-on-mkdocs
branch
from
a788898
to
dbf8ad3
Compare
Codecov Report
@@ Coverage Diff @@
## master #3635 +/- ##
==========================================
+ Coverage 76.75% 76.77% +0.02%
==========================================
Files 158 158
Lines 10048 10049 +1
Branches 1265 1265
==========================================
+ Hits 7712 7715 +3
+ Misses 1995 1994 -1
+ Partials 341 340 -1
|
| <https://docs.readthedocs.io/en/latest/getting_started.html>`_ to become more | ||
| familiar with Read the Docs. | ||
| """ | ||
| ) |
There was a problem hiding this comment.
I also realize that this will look weird on mkdocs (because of the rst specific format used here)
|
I just discovered that Mkdocs already handle the case where a readme file is found, so we can't use the |
|
I think we want to disable all the magic around these cases and force the user to have the same experience locally than in Read the Docs. So, if they don't have a valid index we shouldn't create one. This PR can be closed IMO. |
The thing is that users will get a 404 without any other information. In the generated index file we are showing a message that if they are seeing that page, they should create an index file. |
|
Showing a different 404 page I think makes sense. We should just fix our 404 to link to the FAQ for a couple common cases. That's a different PR though. |
|
I think we should improve our 404 default page instead 👍 |
RTD no longer converts the README to a index file d907524, when a mkdocs project is created without one, is hard to tell why the docs aren't shown (see #1615 and #3321).