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

Use use_directory_urls: false breaks on empty site_url #2189

Closed
ktomk opened this issue Sep 16, 2020 · 36 comments · Fixed by #2405 or #2490
Closed

Use use_directory_urls: false breaks on empty site_url #2189

ktomk opened this issue Sep 16, 2020 · 36 comments · Fixed by #2405 or #2490

Comments

@ktomk
Copy link
Contributor

ktomk commented Sep 16, 2020

When setting the site_url to null (default) and use_directory_urls is false to make the docs browse-able in a user friendly fashion accessible from the local file-system, in the default mkdocs theme the link to the site on the site-name:

<a class="navbar-brand" href=".">{{site_name}}</a>

is missing the /index.html added to the existing href . (dot).

@ktomk
Copy link
Contributor Author

ktomk commented Sep 16, 2020

The HMTL fragment in the report is from base.html, the href value from nav.homepage.url:

<a class="navbar-brand" href="{{ nav.homepage.url|url }}">{{ config.site_name }}</a>

With best intentions, I think this could be fixed in the url filter:

  • Given mkdocs is configured to not use directory urls
  • When the url filter is applied and the url filter is about to return an url-text having the first and the last character Unicode Character 'FULL STOP' (U+002E)
  • Then url-text is appended with /index.html before the url filter returns the url-text.

@squidfunk
Copy link
Contributor

squidfunk commented Oct 4, 2020

This is indeed a bug of MkDocs which should be fixed. We also had this problem as part of Material for MkDocs, which I was able to mitigated with squidfunk/mkdocs-material@454339b – just in case you're using this template. Other than that, the commit may be helpful for other theme devs.

ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 12, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter (... | url).

NOTE: Some values to the filter are declared undefined behavior as the
      filter will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 12, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter (... | url).

NOTE: Some values to the filter are declared undefined behavior as the
      filter will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 12, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter (... | url).

NOTE: Some values to the filter are declared undefined behavior as the
      filter will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 12, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter ( {{ ... | url }} ).

NOTE: Some values to the filter are declared undefined behavior as the
      filter will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
@waylan waylan added the Bug label Oct 28, 2020
@waylan
Copy link
Member

waylan commented Oct 28, 2020

Given the reliance on site_url for some basic functionality, I'm wondering if the config option should be required. I have seen a lot of reports over the years which have all been resolved simply be setting a site_url. If we required it, then users wouldn't miss it. Thoughts?

@squidfunk
Copy link
Contributor

Wouldn't that break local browsing? Some users of Material for MkDocs build their docs and distribute the resulting *.html files as part of their software.

It's great to see you're back, @waylan!

@waylan
Copy link
Member

waylan commented Oct 28, 2020

Wouldn't that break local browsing?

That is a potential concern; except that we have never officially supported local file browsing. For the serve command, we override the setting using the dev_addr. But that doesn't work for local file browsing. Would setting the value to file:// work? If so, that might be a way for users to specifically indicate that they want local file browsing and we could (possibly) alter some behaviors based on that (like perhaps change the default value of use_directory_urls). Yes, that means we could potentially add official support for local file browsing.

I should note that any of the existing solutions for local file browsing are hacks by users. While I don't see a need to break those hacks unnecessarily, any new official way to support such a feature is not likely to work well with the existing hacks.

Finally, the elephant in the room is the builtin search plugin, which does not work with local file browsing and the primary reason why we do not officially support the option. IIRC, the Material theme doesn't have that limitation, but we would need to address that before any official support exists. That said, any changes to the plugin should be discussed in a separate issue. At a minimum, if site_url is set to file:// we might disable the default plugin and require the user to explicitly include a search plugin in their config. In that way, we ensure the user is responsible for providing a working search feature when using local file browsing.

All of the above are just ideas. I havn't looked at the feasibility of any of them. I'm just thinking out load here.

@waylan
Copy link
Member

waylan commented Oct 28, 2020

we could (possibly) alter some behaviors based on that (like perhaps change the default value of use_directory_urls)

That got me thinking... What if we just changed the default of use_directory_urls when site_url is blank? As previously mentioned, the serve command overrides the setting using the dev_addr. But with the build command, site_url remains blank if it is not configured. Without a site_url we could assume the files will not be hosted behind a server. In that case, the default for use_directory_urls should be False. In that way, we can only get a conflict between the two options if the user explicitly sets them in conflict. And we can warn against that in documentation.

The one concern here is that there is a distinct difference in output between serve and build simply by failing to set one optional config option. That's why I keep thinking that the site_url option should be required.

@squidfunk
Copy link
Contributor

squidfunk commented Oct 28, 2020

Finally, the elephant in the room is the builtin search plugin, which does not work with local file browsing and the primary reason why we do not officially support the option. IIRC, the Material theme doesn't have that limitation, but we would need to address that before any official support exists. That said, any changes to the plugin should be discussed in a separate issue. At a minimum, if site_url is set to file:// we might disable the default plugin and require the user to explicitly include a search plugin in their config. In that way, we ensure the user is responsible for providing a working search feature when using local file browsing.

Material relies on mkdocs-localsearch and iframe-worker to support search on file://. A lot of work was put into getting this to work, so it would be quite sad if we'll loose support for local browsing and search. Adding support for local search to the default themes should also be possible with some small additions.

That got me thinking... What if we just changed the default of use_directory_urls when site_url is blank? As previously mentioned, the serve command overrides the setting using the dev_addr. But with the build command, site_url remains blank if it is not configured. Without a site_url we could assume the files will not be hosted behind a server. In that case, the default for use_directory_urls should be False. In that way, we can only get a conflict between the two options if the user explicitly sets them in conflict. And we can warn against that in documentation.

Hard to say whether that won't break anything. It's only another default, so it can be overridden, but it may result in confusion for users.

I'd like to get @wilhelmer in the loop because he's the own maintaining the localsearch plugin.

Another idea:

site_url: null

@waylan
Copy link
Member

waylan commented Oct 28, 2020

Material relies on mkdocs-localsearch and iframe-worker to support search on file://.

I just looked at those for the first time. At a glance, I don't see any reason why that wouldn't work with other themes with a couple minor tweeks. Still, it seems like a lot of work for the user (overriding template blocks) just to enable local file browsing. I wonder if perhaps mkdocs-localsearch could be merged with the search plugin. In fact, before looking at it, I assumed it would be a replacement for the search plugin, not an additional plugin which works beside it. In any event, that is a discussion for a different issue.

So far we have three proposed values for site_url: file://, null and an empty string. The issue with the empty string is that that has been the default for some time. We could change the default behavior in surprising ways for some users who have previously carefully crafted a config in just they way they want. Introducing null provides us a way to introduce a new behavior which won't break any existing configs.

I also have some reservations about file://. Presumably site_url would point at the site root. But for file based browsing, the site root is not the same as the system root. In that case, the value would need to be file:///path/to/site/root and that value could change based upon where within the file system a user happens to have copied the files. Using null avoids that issue.

@squidfunk
Copy link
Contributor

squidfunk commented Oct 28, 2020

In order for search to work locally, the search_index.json is served as search_index.js, which exposes the search_index in window scope. The mechanics are entirely different from the normal way this would be done, as XHR is not available. It could be integrated with the search plugin, sure. That would actually be absolutely amazing, but I think it won't work without explicit theme support, as the search_index.js would need to be included by the theme before the application logic. As this is now a blocking request, performance would degrade for non-local environments, as search_index.js is not loaded asynchronously but is part of the critical rendering path.

Furthermore, if search is implemented as part of a web worker, the iframe-worker polyfill is necessary, too, increasing the payload size even more, albeit it's <1kb gzipped.

@squidfunk
Copy link
Contributor

I also have some reservations about file://. Presumably site_url would point at the site root. But for file based browsing, the site root is not the same as the system root. In that case, the value would need to be file:///path/to/site/root and that value could change based upon where within the file system a user happens to have copied the files. Using null avoids that issue.

This is the reason why I'm not a fan of file:// as a value for site_url because it's inherently dynamic.

@waylan
Copy link
Member

waylan commented Oct 28, 2020

Yeah, let's abandon the file:// suggestion. However, I would be interested to see what we could do with null.

@waylan
Copy link
Member

waylan commented Oct 28, 2020

I just checked and the default value for site_url is null not an empty string as we both assumed above. Here's my proposal:

  1. If the user has explicitly set site_url to an empty string, we should assume the build will be for local file browsing and validate accordingly.
  2. If site_url is undefined by the user we should issue a warning, which will be changed to an error in a later release. This provides a graceful path to making the setting required. If a user has a valid use-case for the value to be null, they can explicitly set that as the value to avoid the warning/future error.
  3. If the user sets site_url and use_directory_urls, in conflicting ways, then we should issue a validation error. In other words, a validation error would be raised if use_directory_urls is True and site_url is either set to an empty string or null. This error should only be raised when at least one of the two settings are explicitly defined by the user. In that way, we avoid this error when the existing default config is used (with neither value being defined). Perhaps in a future version when we change 2 above from a warning to an error, this error could be changed to be raised regardless of whether null was explicitly set or not.

Assuming a user has an existing project and has not defined either setting, upon upgrading to the new version of MkDocs, she would get the warning discussed in 2 above. She could simply ignore the warning and the existing behavior would continue. However, she would want to avoid any future updates.

On the other hand, if, in an effort to avoid the warning, she then sets site_url to a value which conflicts with use_directory_urls, she would get the error discussed in 3 above. At that point, she could either change the value of site_url to something else or properly set use_directory_urls to a non-conflicting value.

For a new user who first creates a project with the new version of MkDocs, the generated config file would include an entry for site_url which the user would be expected to set explicitly. We would provide a sensible default in the generated default config file for a new project. If the user then edits the config file and sets site_url to null or an empty string, and either attempts to set use_directory_urls to True or leaves it as the default value of True by not setting it at all, then they will get the error discussed in 3 above. As with the previous example, this user could either change the value of site_url to something else or properly set use_directory_urls to a non-conflicting value.

But what is a sensible default to put in the generated config file of a new project? We can't explicitly set the value to null or an empty string in the generated file as that would cause the error in 3 above to be raised.

Therefore, we could insert an example domain, such as https://example.com and expect the user to change it. Of course, as serve ignores the value, users who want to wait until later to decide on an actual domain can just leave the default value in place until they are ready to deploy and have domain. And users who are deploying to local file browsing would change the value to an empty string (and also set use_directory_urls to False). A benefit of using https://example.com is that the example value informs users of what is expected to be used as a value of the setting. It should also be a hint to those who want local file browsing that they need to alter some settings to get things to work.

And the documentation for both settings should link to each other and explain the interaction between them. We would include instructions specifically for how to enable local file browsing which explains that both settings need to be set and what values they should be set to.

Finally, note that in this proposal, no defaults have been changed for any settings. I initially started to write this proposal with the idea that the default value for use_directory_urls would change if site_url was set to an empty string, but in the end determined that that was unnecessary and would only add confusion. Therefore, I abandoned that idea.

@squidfunk
Copy link
Contributor

squidfunk commented Oct 29, 2020

I don't feel I'm proficient enough in the internal mechanics of MkDocs to decide whether that covers all cases, but it sounds good to me in general. In essence, we have three cases that should be covered:

  • Web hosting with directory URLs: site_url set and use_directory_urls: true (default)
  • Web hosting without directory URLs: site_url set and use_directory_urls: false
  • Local serving: site_url not set and use_directory_urls: true (must be)

However, there might be cases where people don't set the site_url and use use_directory_urls: true, e.g. when they want to deploy the same documentation on different domains. As I understood, you'd want to issue a warning then, making it impossible to build with --strict. I'd expect some issues rolling in for those cases, as it was the case for #2108.

@waylan
Copy link
Member

waylan commented Oct 29, 2020

A minor grip: site_url should never be "not set." "Not set" means it is not assigned any value in the config file. By making it required, it must be set to some value, which could be null or an empty string.

However, there might be cases where people don't set the site_url and use use_directory_urls: true, e.g. when they want to deploy the same documentation on different domains.

I'll assume you mean people might want to set site_url to null or an empty string. Then yes, that would issue a warning. Strictly speaking, that already results in a broken site (see this issue). In fact, technically speaking, we already require them to have a separate config file for each domain they deploy to with the site_url set to that specific domain.

Note that my solution to this issue (site_name is broken when site_url is null and use_directory_urls is True) is to not allow that combination of settings to exist.

@waylan
Copy link
Member

waylan commented Oct 29, 2020

when they want to deploy the same documentation on different domains

That is only a concern because we don't offer an official way to define multiple deployments within a config. Therefore, users who want that have come up with various hacks. Note my proposal in #2218 which is intended to provide an official option. Of course, please keep discussions about #2218 in that issue. I only mention it here to note that if we implemented a solution to #2218, it would remove that as a blocker for this issue. Especially as it seems that that is the only cited reason to not make site_url a required option.

@squidfunk
Copy link
Contributor

@waylan I’m always a fan of generic solutions that solve a lot of problems at once and #2218 looks pretty promising! Unfortunately, I’m currently a little tight on time, but I’ll try to provide input where possible. Feel free to mention me where it makes sense!

ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 30, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter ( {{ ... | url }} ).

NOTE: Some values to the filter are undefined behavior as the filter
      will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 30, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter ( {{ ... | url }} ).

NOTE: Some values to the filter are undefined behavior as the filter
      will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
ktomk added a commit to ktomk/mkdocs that referenced this issue Oct 31, 2020
Previously when "use_directory_urls" was false and "site_url" empty (e.g.
documentation for fs), the link to the home-page / main index was broken
missing /index.html at the end of the url.

Fix is to add the missing index.html in such a case by the url-template-
filter ( {{ ... | url }} ).

NOTE: Some values to the filter are undefined behavior as the filter
      will return wrong urls. These cases are considered theoretical
      (a file-name must end with a dot and be at the end of value) and not
      likely to happen.
      Declaring them undefined now, allows to change the implementation
      later in a compatible way.
@ktomk
Copy link
Contributor Author

ktomk commented Oct 31, 2020

I have problems to follow this prolonged discussion but would like to bring things forward in a constructive manner.

Who needs to make what kind of design decision?

Answers to that question would help me to continue with this work.

@waylan
Copy link
Member

waylan commented May 12, 2021

In the end, I can still implement my proposal with one change: A null value is not permitted for site_url. I think that is fine. We actually didn't ever have a null value before (if the user set it to null it would be converted to an empty string). Therefore, there is no change in behavior in that case.

@squidfunk
Copy link
Contributor

squidfunk commented May 13, 2021

As long as the built documentation can still be browsed offline and the concerns in #2189 (comment) are nullified, I'm happy with anything that you come up with. You know the inner workings of MkDocs much better than me. Thanks for your work!

waylan added a commit to waylan/mkdocs that referenced this issue May 13, 2021
The site_url config option is now required. If it is set to an empty
string, then use_directory_urls will be forced to false. Each will
issue a warning if not properly set. In a future release we may
raise an error instead.

Fixes mkdocs#2189.
waylan added a commit that referenced this issue May 17, 2021
The site_url config option is now required. If it is set to an empty
string, then use_directory_urls will be forced to false. Each will
issue a warning if not properly set. In a future release we may
raise an error instead.

Fixes #2189.
@MathieuPuech
Copy link

Is there a way to set the site_url with the command line?

@ktomk
Copy link
Contributor Author

ktomk commented May 21, 2021

Is there a way to set the site_url with the command line?

yes, via the -f, --config-file option and its argument. a very simple (unstable) example:

<<MKD mkdocs build -f -                                                                    
docs_dir: $(pwd)/docs
site_url: http://example.com/
site_name: flux
MKD

unstable: if the file grows, mkdocs might become more picky and then requires a regular file (needs seek). a temporary file then should do it (e.g. =(...) in zsh).

mind the docs_dir if not specified within the file.

(comment against mkdocs version 1.1.2)

@waylan
Copy link
Member

waylan commented May 22, 2021

Is there a way to set the site_url with the command line?

In the 1.2 release (which will require a site_url) the easiest way to do this would be through an environment variable, which is another new feature of 1.2. See more here.

First, use the !ENV tag in the config file:

site_url: !ENV [SITE_URL, https://fallback.com]

Then on the commandline do:

SITE_URL="https://example.com" mkdocs build

@MathieuPuech
Copy link

May I ask why you need the complete url of the website to build the documentation website?

If I could do

site_url: /

I could upload the same build on different hosts if it was possible.

The current error I get when I have this error logged:

ERROR   -  Config value: 'site_url'. Error: The URL isn't valid, it should include the http:// (scheme)

NB: I ask about this here because it's the issue where it was discussed to make site_url mandatory when use_directory_urls is true

br3ndonland added a commit to br3ndonland/inboard that referenced this issue Jul 5, 2021
mkdocs/mkdocs#2189

```text
WARNING  -  Config value: 'site_url'.
            Warning: This option is now required. Set to a valid URL
            or an empty string to avoid an error in a future release.
```
br3ndonland added a commit to br3ndonland/inboard that referenced this issue Jul 7, 2021
#34
36eca9d
a877045

https://www.mkdocs.org/user-guide/configuration/#site_url
https://www.mkdocs.org/user-guide/configuration/#use_directory_urls
mkdocs/mkdocs#2189
mkdocs/mkdocs#2360

https://squidfunk.github.io/mkdocs-material/setup/setting-up-navigation/
squidfunk/mkdocs-material#2520

MkDocs recently made the `site_url` setting required. A warning is now
seen when `site_url` is not included in mkdocs.yml:

```text
WARNING  -  Config value: 'site_url'.
            Warning: This option is now required. Set to a valid URL
            or an empty string to avoid an error in a future release.
```

The docs describe the `site_url` setting as follows:

> Set the canonical URL of the site. This is a required setting. If the
> 'root' of the MkDocs site will be within a subdirectory of a domain,
> be sure to include that subdirectory in the setting
> (https://example.com/foo/). If the domain is yet to be determined, you
> may use a placeholder domain, which will need to be updated prior to
> deployment.
>
> If the built site will not be behind a server, then you may set the
> value to an empty string (''). When set to an empty string, some
> features of MkDocs may act differently. For example, the
> `use_directory_urls` setting must be set to false.

If the docs are "behind a server," they often have more than one URL.
Which should be set as the `site_url`? Furthermore, if the related
`use_directory_urls` is set to `true`, it appends a trailing slash to
the URLs generated for the ToC (Table of Contents).

This commit will set `site_url: ""` and `use_directory_urls: false`.
In combination with the `{"cleanUrls": true, "trailingSlash": false}`
settings in vercel.json, the previous behavior of URLs and navigation
will be restored.
br3ndonland added a commit to br3ndonland/fastenv that referenced this issue Jul 7, 2021
#1
br3ndonland/inboard#34
br3ndonland/inboard@9943112

https://www.mkdocs.org/user-guide/configuration/#site_url
https://www.mkdocs.org/user-guide/configuration/#use_directory_urls
mkdocs/mkdocs#2189
mkdocs/mkdocs#2360

https://squidfunk.github.io/mkdocs-material/setup/setting-up-navigation/
squidfunk/mkdocs-material#2520

MkDocs recently made the `site_url` setting required. A warning is now
seen when `site_url` is not included in mkdocs.yml:

```text
WARNING  -  Config value: 'site_url'.
            Warning: This option is now required. Set to a valid URL
            or an empty string to avoid an error in a future release.
```

The docs describe the `site_url` setting as follows:

> Set the canonical URL of the site. This is a required setting. If the
> 'root' of the MkDocs site will be within a subdirectory of a domain,
> be sure to include that subdirectory in the setting
> (https://example.com/foo/). If the domain is yet to be determined, you
> may use a placeholder domain, which will need to be updated prior to
> deployment.
>
> If the built site will not be behind a server, then you may set the
> value to an empty string (''). When set to an empty string, some
> features of MkDocs may act differently. For example, the
> `use_directory_urls` setting must be set to false.

If the docs are "behind a server," they often have more than one URL.
Which should be set as the `site_url`? Furthermore, if the related
`use_directory_urls` is set to `true`, it appends a trailing slash to
the URLs generated for the ToC (Table of Contents).

This commit will set `site_url: ""` and `use_directory_urls: false`.
In combination with the `{"cleanUrls": true, "trailingSlash": false}`
settings in vercel.json, the previous behavior of URLs and navigation
will be restored.
@oprypin
Copy link
Member

oprypin commented Jul 8, 2021

I intend to revisit (i.e. likely revert) the decision to make site_url mandatory.
Input on the matter is welcome.

@oprypin oprypin reopened this Jul 8, 2021
@ktomk
Copy link
Contributor Author

ktomk commented Jul 8, 2021

@oprypin Can you add some context? As long as it transparently works, I'm not against it (as I had the reported case). Currently it is "patched" in the theme and when it became mandatory, I was injecting it IIRC, I have tooling around anyway so this was not that much of a deal for me personally.

@oprypin
Copy link
Member

oprypin commented Jul 8, 2021

Well it's a big deal for me and at least some people
-- see these comments with some thumbs up
#2189 (comment)
#2479

Basically the argument is that a URL is not something inherent to a collection of documents.
It was also a breaking change that, as far as I saw, was not sufficiently justified.
I would like to just fix the bug as it was originally reported -- and don't we already have the fix in #2202?

@ktomk
Copy link
Contributor Author

ktomk commented Jul 8, 2021

I would like to just fix the bug as it was originally reported -- and don't we already have the fix in #2202?

That sounds good to me. And yes, there was a fix, it's a bit older now, but from my end I gave it quite some love and testing. But mind, even I'm doing URL related stuff since quite a long time, I'm not at all that long familiar with mkdocs. So I guess the main decision reg. #2202 is likely a design decision how to handle this.

And IIRC for #2202 I got tests running but I'm not fully confident my unit-test setup was right. And also part of the existing code perhaps was missing coverage and I added it, likely before the changes but it's perhaps worth to replay this.

@oprypin
Copy link
Member

oprypin commented Jul 8, 2021

@ktomk Thanks and don't worry about that. I'll look at it first. At the very least, don't rebase #2202 on the current master :)

@oprypin

This comment has been minimized.

@oprypin
Copy link
Member

oprypin commented Jul 8, 2021

Nevermind, my comment is wrong.
Basically use_directory_urls false isn't even expected to work at all, which is why it's easy to find problematic cases, while true is supposed to work and there is one edge case.

Looking at this commit squidfunk/mkdocs-material@454339b clarifies things more than any textual description :)

@oprypin
Copy link
Member

oprypin commented Jul 8, 2021

I found that this issue was actually fixed by #2366, and neither #2202 nor #2405 are necessary.

$ git checkout -f 1.1.2                                    

$ echo $'site_url: null\nuse_directory_urls: false' >>mkdocs.yml; mkdocs build; grep 'navbar-brand' site/index.html site/about/license.html
site/index.html:                <a class="navbar-brand" href=".">MkDocs</a>
site/about/license.html:                <a class="navbar-brand" href="..">MkDocs</a>

$ git checkout -f d038223e7ab52ba6cd8890909d97645856ba6b31~

$ echo $'site_url: null\nuse_directory_urls: false' >>mkdocs.yml; mkdocs build; grep 'navbar-brand' site/index.html site/about/license.html
site/index.html:                <a class="navbar-brand" href=".">MkDocs</a>
site/about/license.html:                <a class="navbar-brand" href="..">MkDocs</a>

$ git checkout -f d038223e7ab52ba6cd8890909d97645856ba6b31 

$ echo $'site_url: null\nuse_directory_urls: false' >>mkdocs.yml; mkdocs build; grep 'navbar-brand' site/index.html site/about/license.html
site/index.html:                <a class="navbar-brand" href="index.html">MkDocs</a>
site/about/license.html:                <a class="navbar-brand" href="../index.html">MkDocs</a>

@ktomk Could you please git checkout d038223 and see if you can still see any manifestation of issue(s) discussed here? (and the immediately preceding commit is supposed to be broken still)

@ktomk
Copy link
Contributor Author

ktomk commented Jul 13, 2021

@oprypin: If I finally manage to run a test ;), should I run it against master then? Or against the merge commit?

@oprypin
Copy link
Member

oprypin commented Jul 13, 2021

@ktomk I said that you could test on d038223 and that still applies. But yes, now that the revert is merged, it's even better to test on master.

@oprypin
Copy link
Member

oprypin commented Jul 13, 2021

@MathieuPuech Thanks for the suggestion #2189 (comment)
In #2503 I want allow specifying just the path, not the whole URL. Of course, site_url will stop being mandatory anyway, so you probably don't care so much about that ability anymore, but it gave me an idea :)

@ktomk
Copy link
Contributor Author

ktomk commented Nov 8, 2021

For the file:// based static site I'm now using the material theme solution:

extra:
  homepage: index.html

which works. I was not able to get it working with vanilla mkdocs config (e.g. empty site_url with directory_urls: false ). Just FYI in case its helpful for anyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment