Remove doctype from resolver #5230
Conversation
stsewd
force-pushed
the
remove-doctype-from-resolver
branch
from
fc8d84c to
1c068f0
Compare
|
Also, our current logic is making wrong assumptions about mkdocs projects https://www.mkdocs.org/user-guide/configuration/#use_directory_urls |
| path = filename + '.html' | ||
| else: | ||
| path = '' | ||
| return path |
There was a problem hiding this comment.
I do feel like we introduced this for a reason, but perhaps we've removed that path from the codebase or something. I agree that the less we do with guessing URL's for things, the better, but I could still see this breaking in a random place.
There was a problem hiding this comment.
So, what I was thinking, this is used to reverse the url results from search, so all urls are present in the same format as the docs are build, rather than always including .html/index.html
There was a problem hiding this comment.
That's the only place where I can think it could break something, but when indexing from search we save the whole file, with the extension, no need to reverse anything from there. The other places we really don't use this at all.
There was a problem hiding this comment.
I'm still a little worried that this might break things, but I don't have a good way of testing it beyond just deploying it. @agjohnson do you have any memory of why we added this?
There was a problem hiding this comment.
@stsewd if you feel like this has been tested enough, I'm fine merging it and seeing if anything blows up in prod :)
|
Ok, couldn't find anything breaking. Just more bugs related to the doctype thing that are present in master :) @ericholscher can you approve this again to merge? |
Total explanation why this is safe to remove in:
#4638 (comment)
#4638 (comment)
Ref #4638