-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix to allow an empty repo_url
when still specifying a edit_uri
#1874
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
Fix to allow an empty repo_url
when still specifying a edit_uri
#1874
Conversation
mkdocs/config/config_options.py
Outdated
@@ -308,6 +308,7 @@ def post_validation(self, config, key_name): | |||
# ensure a well-formed edit_uri | |||
if edit_uri: | |||
if not edit_uri.startswith(('?', '#')) \ | |||
and config['repo_url'] is not '' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change fixes the problem we experienced, I modified the above conditionals as well since they were unreachable before.
Note that Regardless of the above, how are you using |
Fair point! I think our current configuration may just be a misunderstanding of the documentation. As for how we are using To make this config work we had Seems like it was just dumb luck that this worked at all since it wasn't an intentional mkdocs feature haha. Right now we are considering removing the link to the main project as we want the page to be more user focused, but it would be nice to keep the While the defaults are nice if the docs share the same repo as the main project, our docs are more user specific and I feel like conceptually it's easier to manage it as a separate repository (can keep all documentation related PR's in one space separate from main project PR's for instance). Given this isn't an intentional feature though, I think the better approach may be to just fork the theme we're using and customize it to follow our needs. Unless you think this is a valid use-case you want to support, then I would be happy to help develop it with documentation + testing. |
Ah, your documentation and code are in separate repos. I hadn't considered that scenario when reading your initial report.
That seems like a more reasonable approach to me. As you note, MkDocs expects the documentation to be in the same repo as the code. If you are doing something else, then you'll need to do some customization to make that work. Note that instead of forking a theme, you may be able to address this with a plugin which modifies the settings after the builtin validation completes (via the on_config event). That might even make for a popular general-use third party plugin as I'm sure many users have (or would like to have) their code and documentation in separate repos. If there are any changes we need to make to MkDocs to make this easier (so long as the documented behavior is retained), please let us know. |
Perhaps I should mention that we had tentatively planned to break the existing repo related feature out into a plugin which would be more feature complete and support more use cases. We just haven't gotten to it yet. If anyone is up for the work, I'm happy to provide guidance. If anyone is interested in developing and maintining it as a third party plugin, even better. |
Good call, and thank you for taking the time to respond and giving me some direction. I think the plugin approach is preferable for sure.
Do you have a link to this discussion, or can point to a summary? I'd be interested in seeing what the other use cases may be outside of a split repo that isn't covered by the current config. I think short term I will look at developing a 3rd party plugin to cover our use case, but I wouldn't be against trying to cover the other needs with the same plugin long term. |
See this comment for my proposed solution. although the entire discussion is relevant. The primary issue is that the current system doesn't work with all services or for all possible situations. By using a template based URL, any arrangement of URL parts could be defined by the user which is required for any situation a user might have. |
It seems that #2733 happens to fix the issue that |
Generally with mkdocs you can exclude the link to the code repository by omitting the
repo_url
from themkdocs.yml
file.This does work in most cases. One exception to this is if you also have a
edit_uri
config value.We have a MKDoc page where we don't specifically want to point a user to a specific repo, but we do want to encourage contributions to the docs.
If we provide an explicit
edit_uri
likeedit_uri: https://github.com/mkdocs/mkdocs/blob/master/docs/
this works as expected even without arepo_url
.But, because of some logic in
config_options.py
and with this configuration,repo_url
ends up being"/"
which causes our theme to include empty whitespace where the repo link should be (see screenshots).RepoURL
extends fromURL
which has a default value of""
. Because of this, it's impossible forrepo_url
to ever beNone
. At most it will be the empty string""
.This change modifies
RepoURL
to check for this empty string state instead ofNone
, it also adds a test-case for a config with aedit_uri
and emptyrepo_url
.After the change, the template works as expected:
