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

Static resources (especially CSS/JS!) shouldn't be automatically included #986

Closed
jimporter opened this issue Jun 30, 2016 · 16 comments · Fixed by #1020
Closed

Static resources (especially CSS/JS!) shouldn't be automatically included #986

jimporter opened this issue Jun 30, 2016 · 16 comments · Fixed by #1020
Labels
Milestone

Comments

@jimporter
Copy link
Contributor

(Pulling this out from #972.)

Currently, MkDocs automatically includes all static resources from the docs dir in a build. This can cause issues if some files aren't meant to be included but are located in the docs dir. For example, this could happen with files that get preprocessed by another tool into .md files for MkDocs (which was how I planned on doing auto-generation of docs from code for MkDocs if I ever get around to it).

However, there's a more-significant issue: all CSS and JS files are automatically included in the generated HTML itself (except when hosting on RTD, apparently). This means that any CSS/JS used for static HTML files will get erroneously included in the MkDocs-generated HTML by default. The inconsistency with RTD adds another wrinkle to this. I think the behavior should be the same no matter what, and as Python says, "Explicit is better than implicit."

At minimum, I think CSS and JS shouldn't be automatically added to the generated HTML, but I'd prefer to see MkDocs only include files that are explicitly listed in mkdocs.yml. Doing so would make things more flexible when people want to do unusual things with MkDocs, and with globbing or the ability to include whole subdirectories of static resources, the configuration wouldn't be much more complex than it is now.

@waylan
Copy link
Member

waylan commented Jun 30, 2016

I have to admit that I have always found the the extra_css and extra_javascript settings to be odd (same for extra_templates, although that works slightly differently). We provide a way for users to insert their own CSS and JavaScript from the theme_dir, but we also allow the same thing through these "extra" settings from the docs_dir. Why are we offering users two ways to accomplish the same thing?

I'm guessing the answer is historical reasons. My assumption is that the "extra" settings were added first and the theme_dir came later (I haven't actually checked the commit history). But if the "extra" settings are there only for historical reasons, perhaps they should be deprecated altogether. Especially now that the theme_dir is being more fully documented and its use and purpose more fully defined (see #911 & #947 to be available in version 0.16). And, hey, it meets our goal of reducing the number of settings.

Suppose for a moment that I wanted to host a JavaScript file on my site for my readers to download as a separate JavaScript file. For the sake of argument, suppose I have a good reason for this file not being hosted elsewhere (such as a git repo). My first inclination would be to just include it next to my Markdown file which describes and links to it--the same way an image or PDF would work. But instead, the file gets added to extra_javascript and loaded as a script on the page. Maybe nothing breaks, but is my file even at the same path now? Or maybe is does break things by interfering with some script that is supposed to be on the page (variable name collision, etc...)

I see two possible solutions:

  1. Leave the "extra" settings in place, but make them not auto-populating. Only scripts, and CSS files that the user explicitly lists get included. In other words, make extra_css and extra_javascript work the same way extra_templates does now. I believe this is @jimporter's proposal.
  2. Deprecate the extra settings and require users to provide custom CSS and JavaScript via the theme_dir only. I would also suggest deprecating extra-templates.

Both solution's are backward incompatible for any user who relies on these settings being auto-populated. Personally, if we are going to make a change, I would suggest going all the way with option 2. And now is the time to do it as we approach version 1.0.

For completeness, I should mention that some other static site generators have solved this type of problem by using a naming convention for "ignored" files. For example, any file or directory with a name that starts with an underscore is ignored. The settings file, the theme_dir, and the site_dir can then all be nested inside the docs_dir as long as their names all start with an underscore. Personally, I found is refreshing when I first realized MkDocs does not use that model, but I have to admit that is would make some of this stuff easier. But do we really want our static resources to all be in files that start with underscores? Uhg.

@jimporter
Copy link
Contributor Author

For what it's worth, I use the extra_css to add a few minor styling things that I reference in Markdown via the Attributes extension. It looks like I could do this via customizing the styles block in the theme, but that's a bit more work since I think I'd have to copy over all the default style tags into my override. The extra_css is nice since it's a low-friction way of adding a few custom rules to every file.

One way of handling ignored files would be to pick a reasonable default for static files to include (maybe everything that doesn't start with a dot?), but allow users to override this behavior by passing a set of files/globs to include/exclude. For instance:

static_resources:
  - include:
    - sample.txt
    - images/*.png
  - exclude:
    - _*

This would make it possible for people to pick what gets included or not while still retaining the default behavior of including everything. I would strongly prefer it if files were only included if you explicitly listed them, but this would be an ok compromise. In any case, the current strategy of automatically adding all CSS/JS files to the theme seems a bit too error-prone to me.

I'd also encourage a way of warning users if MkDocs is including files in the build that aren't explicitly referenced in mkdocs.yml. This doesn't have to be on by default, though. It could be part of a "strict" mode in MkDocs to help prevent mistakes when building your docs. I know I'd enable that for all my docs builds (but then, I'm the sort of person who always compiles with -Wall -Wextra -Werror -pedantic in GCC).

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

The inconsistency with RTD adds another wrinkle to this.

RTD adds their own layer of restrictions by overwriting settings and they add theme customisations. We can't really control what they do with MkDocs.

@waylan
Copy link
Member

waylan commented Jun 30, 2016

It looks like I could do this via customizing the styles block in the theme, but that's a bit more work since I think I'd have to copy over all the default style tags into my override.

All you need to do is create a CSS file in your theme_dir (probably in a css sub- directory), and that file gets included with the Template files.

Although, in retrospect, it wouldn't get linked from the HTML. But you can use a Super Block to avoid replicating the entire styles block:

{% block styles %}
    {{ super() }}
    <link href="{{ base_url }}/css/my-custom-styles.css" rel="stylesheet">
{% endblock %}

We should probably add that to the documentation somewhere.

Granted, that is more involved than adding an item to the extra_css setting in the config file. Personally, I am for deprecating the "extra" settings altogether, but I understand if the decision is only to make them not auto-populate.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

I'm guessing the answer is historical reasons. My assumption is that the "extra" settings were added first and the theme_dir came later (I haven't actually checked the commit history). But if the "extra" settings are there only for historical reasons, perhaps they should be deprecated altogether.

I'm not sure, I think it was this way when I first got involved in the project.

  1. Leave the "extra" settings in place, but make them not auto-populating. Only scripts, and CSS files that the user explicitly lists get included. In other words, make extra_css and extra_javascript work the same way extra_templates does now. I believe this is @jimporter's proposal.

I think this is the option I would choose. Removing the option entirely makes adding small tweaks quite a bit harder - as you then need a theme_dir and an html file. The overhead and understanding required to then add some CSS is much higher.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

One way of handling ignored files would be to pick a reasonable default for static files to include (maybe everything that doesn't start with a dot?), but allow users to override this behavior by passing a set of files/globs to include/exclude. For instance:

We already skip any files starting with .. I am not sure if this is documented.

@waylan
Copy link
Member

waylan commented Jun 30, 2016

Removing the option entirely makes adding small tweaks quite a bit harder - as you then need a theme_dir and an html file. The overhead and understanding required to then add some CSS is much higher.

I can live with that. This issue has been bothering me from the beginning and I've been able to manage without saying anything so far. I just don't have to use it myself.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

I think we are really talking about two things again - but this time they are probably related enough to be in the same issue.

  1. The issue of automatically populating extra_css and extra_javascript. Sure, let's stop that. We should add a warning in 0.16 if any files are added automatically, then in 1.0 they will need to be specified manually.
  2. Copying of all other static assets. I really don't have a problem with this, I feel like everything I put in my docs_dir should be included. I'd much rather a simpler barrier like this than having to look at a mkdocs.yml to see what is copied over.

@jimporter
Copy link
Contributor Author

Yeah, I'm happy to compromise and leave (2) as it currently is so long as (1) gets fixed. However, I personally prefer being very explicit about what gets included in my docs, so I'd certainly be happy to see (2) fixed as well.

That said, I don't have any current need for (2), so it's really just an academic question for me right now.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

It's interesting to note, that many people are already specifying the files. This may be due to the difference on RTD. Unfortunately there isn't an easy way to search for users that are using it implicitly.

@waylan
Copy link
Member

waylan commented Jun 30, 2016

We already skip any files starting with .. I am not sure if this is documented.

I always forget about that. I wonder if people who really want their site_dir nested in their docs_dir can make that work by naming their site_dir starting with a period? Never tried that myself.

Copying of all other static assets. I really don't have a problem with this, I feel like everything I put in my docs_dir should be included. I'd much rather a simpler barrier like this than having to look at a mkdocs.yml to see what is copied over.

I agree here.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

That said, I don't have any current need for (2), so it's really just an academic question for me right now.

This is basically how I feel about it. If we can come up with a compelling use-case then I would be more interested in the effort required for a disruptive change.

It is interesting to note that extra_templates is a feature that doesn't see much use, it doesn't auto-populate either.

@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

I always forget about that. I wonder if people who really want their site_dir nested in their docs_dir can make that work by naming their site_dir starting with a period? Never tried that myself.

haha, yes, I think that would work. I really don't want to encourage this however, I think that could be highly confusing 😄

@d0ugal d0ugal added this to the 0.16 milestone Jun 30, 2016
@d0ugal
Copy link
Member

d0ugal commented Jun 30, 2016

Okay, setting milestone 0.16 to add a warning about css or javascript that is automatically included in the HTML. We can then have a think about copying assets in general.

@memeplex
Copy link
Contributor

I'm using an instance of mkdocs as a personal wiki. I like the search and categorization capabilities but, first of all, I like mkdocs autopopulating from any md file I throw into docs without further effort. I understand this is not the main use case for mkdocs, but I would be sad to lose that feature. It's ok for other assets not to autopopulate, though.

@d0ugal
Copy link
Member

d0ugal commented Aug 2, 2016

I'm using an instance of mkdocs as a personal wiki. I like the search and categorization capabilities but, first of all, I like mkdocs autopopulating from any md file I throw into docs without further effort. I understand this is not the main use case for mkdocs

I would argue this is one of the main use cases for MkDocs. Allowing people to easily write good documentation and then easily present that to users is probably one of the top goals for the project.

However, I don't think anyone is suggesting removing the automatic inclusion of Markdown files, if you don't define which to include in the config. This issue is about all the related files, like images or css or html

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.

4 participants