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

Theme files override docs_dir files on Windows #1876

Closed
wilhelmer opened this issue Oct 7, 2019 · 3 comments · Fixed by #1878
Closed

Theme files override docs_dir files on Windows #1876

wilhelmer opened this issue Oct 7, 2019 · 3 comments · Fixed by #1878
Labels

Comments

@wilhelmer
Copy link
Contributor

wilhelmer commented Oct 7, 2019

As investigated and discussed in squidfunk/mkdocs-material#1282, custom files in docs_dir are overriden by theme files.

For example, if you use the default MkDocs theme and create a custom favicon.ico in docs/img, the custom icon does not appear in the output. It is overwritten by the theme's own favicon.

The issue was already brought up in #1671 and fixed by #1672.

However, the fix (#1672) only works on Unix systems (including Mac). It does not work on Windows.

Windows handles path names differently, so I suppose the issue must be caused by that. See especially the use of os.path in files.py (f8ac3c7ee).

I had a similar issue in mkdocs-exclude, see here: apenwarr/mkdocs-exclude#2

@waylan
Copy link
Member

waylan commented Oct 7, 2019

At first I was confused by this because we have a test (added in f8ac3c7) which passes on all supported systems (including Windows). But then I realized that the test does not include any nested directories. All paths tested do not include any slashes (for example, favicon.ico is the complete path tested). However, the failing example in squidfunk/mkdocs-material#1282 is using a nested path (assets/images/favicon.png). Therefore, if line 77 is comparing assets/images/favicon.png to assets\images\favicon.png, the comparison will fail and the file will be overwritten. Normally, this is not a problem as MkDocs uses the same methods for creating all of its paths internally, which ensures that they all use the same format. However, in this instance, one of the paths being compared is provided by the Jinja template environment and we cannot assume that that path is normalized in the same manner,

This is my best guess without a way to test it. To make matters worse, our Windows CI tests are broken for reasons I don't understand, so I can't rely on them for testing.

@wilhelmer
Copy link
Contributor Author

wilhelmer commented Oct 7, 2019

Normalizing the path before checking the files in files.py fixes it:

for path in env.list_templates(filter_func=filter):
    # Theme files do not override docs_dir files
    path = os.path.normpath(path)
    if path not in self:
        for dir in config['theme'].dirs:
            # Find the first theme dir which contains path                   
            if os.path.isfile(os.path.join(dir, path)):
                self.append(File(path, dir, config['site_dir'], config['use_directory_urls']))
                break

(Line 3 was added.)

Didn't test it on Unix, but should work there as well.

@waylan waylan added the Bug label Oct 7, 2019
@waylan
Copy link
Member

waylan commented Oct 7, 2019

That is exactly what I suspected the fix would be. Thanks for testing it. Feel free to create a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants