Fix custom_dir support and minification inconsistency

[kamilkrzyskow]

Hello again,

my team started supporting multiple languages in the docs, and I moved the shared extra assets to our overrides directory to avoid file duplication. This created an issue with the cache_safe option. Seems like people don't use this setting, because otherwise this issue would be found sooner. Maybe people prefer less "invasive" ways like this one, however I'm not sure if that's all there is to it.

Additionally, I've unified the minification when using and not using the cache_safe option.
The order of functions makes the code still hard to read and maybe has caused the bug in the first place
My attempt at restructuring wasn't accepted, because I did too many things back then #19 and then opted into a less invasive revision, but I still think it's worth a shot.

You could add some "easy-first-commit" issues to:

• add missing tests for different cases of JavaScript files that would test them to make sure the options are correctly selected.
• simplify cache busting - however it's supposed to be added in MkDocs 1.5.0, once it releases, so I'm not sure if it's worth giving more thought into a soooon™️ to be deprecated option
• restructure code to make it easier to read and flow similarly to the event chart

[kamilkrzyskow]

No feedback and no merge, and I've seen the other PR merged 🤔Am I supposed to change something here @wilhelmer?

[wilhelmer]

To be honest, I don't know what to do with this. What's the issue with the custom_dir? Why would you minify anything inside the custom theme directory? Also, I'm only a super bad wannabe developer, so I can't say anything regarding missing tests or code restructuring. Maybe @byrnereese can help here.

[kamilkrzyskow]

This PR is safe to merge, it introduces no breaking changes for the end-user.
The remarks at the end are just suggestions if you want the project to be open for "easy-first" commits.
I can add some test cases if you don't want to wait for willing beginners, just ask 😉.

The path in my mkdocs.yml is the same as before:

  - minify:
      minify_html: !ENV [GMC_ENABLE_ON_PUBLISH, False]
      minify_css: !ENV [GMC_ENABLE_ON_PUBLISH, False]
      minify_js: !ENV [GMC_ENABLE_ON_PUBLISH, False]
      cache_safe: !ENV [GMC_ENABLE_ON_PUBLISH, False]
      htmlmin_opts:
        remove_comments: true
      css_files:
        - assets/stylesheets/extra.css
      js_files:
        - assets/javascripts/extra.js

The difference is that I moved all of my shared assets into the overrides directory:

and then it crashed in open():

mkdocs-minify-plugin/mkdocs_minify_plugin/plugin.py

Lines 148 to 150 in cecdab8

	docs_file_path: str = f"{config['docs_dir']}/{file_path}".replace("\\", "/")
	with open(docs_file_path, encoding="utf8") as file:

because it expected that the file will be located within the docs_dir, but right now it's not the case, so I fixed it by finding the first valid file path in a custom_dir of the theme aka the overrides directory in my case.

The second issue was that the new js_min handling introduced in a previous commit was only working when the cache_safe setting was enabled:

mkdocs-minify-plugin/mkdocs_minify_plugin/plugin.py

Lines 147 to 157 in e0ab4d9

	if self.config["cache_safe"]:
	docs_file_path: str = f"{config['docs_dir']}/{file_path}".replace("\\", "/")
	with open(docs_file_path, encoding="utf8") as file:
	file_data: str = file.read()
	if minify_flag:
	if minify_func.__name__ == "jsmin":
	file_data = minify_func(file_data, quote_chars="'\"`")
	else:
	file_data = minify_func(file_data)

and you probably want to handle the js_min the same way even if the cache_safe setting is disabled 😄.
The error was hidden because the current tests didn't properly check JavaScript minification with different settings enabled.

[oprypin]

It caught me off guard that someone is actually using user_configs in MkDocs. I thought it's there in MkDocs only by accident. Could you explain why it was used here, I'm curious. I thought that one could just read config.theme directly and get the same outcome.

[kamilkrzyskow]

I used it since otherwise I would have to use config.theme._vars, and since it's a pythonic-private variable I preferred to use a public one.

I did the same in the social cards plugin in MkDocs Material.