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

Allow docs to be in the root directory #3519

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Allow docs to be in the root directory #3519

wants to merge 3 commits into from

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Dec 15, 2023

If the config file is inside docs_dir, you get this message:

ERROR   -  Config value 'docs_dir': The 'mkdocs.yml' config file should not be inside the docs_dir.
           To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration:

           exclude_docs: |
             /mkdocs.yml

If the site_dir is inside docs_dir, you get this message:

ERROR   -  Config value 'site_dir': The site_dir should not be inside the docs_dir as this leads to the build directory being copied into itself.
           To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration:

           exclude_docs: |
             /site

Fix bypassing it when:

* a non-absolute config_file_path is passed on the command line
* the directory is not an *exact* parent of the other one, but is instead a "grandparent".

Also validate it in `mkdocs serve` because why reserve the check only to `mkdocs build`.
@oprypin
Copy link
Contributor Author

oprypin commented Dec 15, 2023

My motivation for this is that the user will be informed about the setting and will take this opportunity to check which other files should perhaps be excluded as well.

@athackst
Copy link
Contributor

I think the site_dir message is good.

I'm not sure I follow what use case you're protecting against to warn against the mkdocs.yml file inside of the docs dir.

Previously mkdocs required the docs dir to be located in a child directory named 'docs', but since you can now specify the config from the command line, I don't believe this is the case any more?

@oprypin
Copy link
Contributor Author

oprypin commented Dec 17, 2023

Well, do people want to publish mkdocs.yml online? Probably not? So how can it be a bad warning. And I am hoping that people will take this opportunity to also exclude all other kinds of files that may be in the root directory of their repository.

@athackst
Copy link
Contributor

Ah, ok. Makes sense then.

@athackst
Copy link
Contributor

Would you consider making it a warning instead of an error?

@oprypin
Copy link
Contributor Author

oprypin commented Dec 20, 2023

Error is closer to the current behavior than no error, so it was the default choice. Currently I don't see strong arguments in either direction. It can always be changed to warning, and could even be changed now, sure. Do you have any use case in mind?

@athackst
Copy link
Contributor

Yeah, if you want to do something like #3526 so excluded docs aren't triggering the reload server but you want to have mkdocs.yml trigger it, you might want to not add it to exclude_docs

@athackst
Copy link
Contributor

Eh, nevermind. I think I found a clean way of handling that case. An error sounds fine to me.

@oprypin
Copy link
Contributor Author

oprypin commented Apr 21, 2024

I didn't merge this because not sure if next maintainers will want to own this code.

@athackst
Copy link
Contributor

Who are the next maintainers?

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Apr 21, 2024

Right now, they are the people in the @mkdocs/core team who have the time and will to work on MkDocs :) As you know it, we're also trying to onboard more people.

@oprypin
Copy link
Contributor Author

oprypin commented Apr 21, 2024

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