fralau / mkdocs-macros-plugin

Create richer and more beautiful pages in MkDocs, by using variables and calls to macros in the markdown code.
https://mkdocs-macros-plugin.readthedocs.io
Other
321 stars 50 forks source link

missing macro fix_url #87

Closed CrispyDrone closed 3 years ago

CrispyDrone commented 3 years ago

Hello,

My build pipeline suddenly started failing. It seems to be related to the fix_url macro not being found in the variables property of the plugin.

This is for version 0.5.12 of mkdocs-macros-plugin. Reverting to 0.5.5 fixes the issue.

Next, I went through the code and it seems the fix_url function is now part of a macros property instead of variables? Is this not a breaking change? Upgrading once more to 0.5.12 and using env.macros.fix_url is working.

This is (was) my full main.py:

import os

def define_env(env):

    # import the predefined macro
    fix_url = env.variables.fix_url # make relative urls point to root

    @env.macro
    def button(label, url):
        "Add a button"
        url = fix_url(url)
        HTML = """<a class='button' href="%s">%s</a>"""
        return HTML % (url, label)

    @env.macro
    def pages(path):
        "Returns filepaths of files in a directory"
        return sorted(os.listdir(os.path.dirname(path)))

Log output from my build server:

2021-06-25T17:48:31.3483372Z Traceback (most recent call last):
2021-06-25T17:48:31.3484141Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs_macros\util.py", line 158, in __getattr__
2021-06-25T17:48:31.3513942Z     return self[name]
2021-06-25T17:48:31.3514463Z KeyError: 'fix_url'
2021-06-25T17:48:31.3514677Z 
2021-06-25T17:48:31.3515207Z During handling of the above exception, another exception occurred:
2021-06-25T17:48:31.3517181Z 
2021-06-25T17:48:31.3518947Z Traceback (most recent call last):
2021-06-25T17:48:31.3519927Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\runpy.py", line 197, in _run_module_as_main
2021-06-25T17:48:31.3520703Z     return _run_code(code, main_globals, None,
2021-06-25T17:48:31.3521620Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\runpy.py", line 87, in _run_code
2021-06-25T17:48:31.3522310Z     exec(code, run_globals)
2021-06-25T17:48:31.3523025Z   File "E:\TfsBuildAgents\Default\BuildAgent1\_work\123\s\python\tools\scripts\mkdocs.exe\__main__.py", line 7, in <module>
2021-06-25T17:48:31.3528044Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\click\core.py", line 1137, in __call__
2021-06-25T17:48:31.3565032Z     return self.main(*args, **kwargs)
2021-06-25T17:48:31.3565826Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\click\core.py", line 1062, in main
2021-06-25T17:48:31.3570519Z     rv = self.invoke(ctx)
2021-06-25T17:48:31.3571269Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\click\core.py", line 1668, in invoke
2021-06-25T17:48:31.3578048Z     return _process_result(sub_ctx.command.invoke(sub_ctx))
2021-06-25T17:48:31.3578942Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\click\core.py", line 1404, in invoke
2021-06-25T17:48:31.3585201Z     return ctx.invoke(self.callback, **ctx.params)
2021-06-25T17:48:31.3586095Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\click\core.py", line 763, in invoke
2021-06-25T17:48:31.3589913Z     return __callback(*args, **kwargs)
2021-06-25T17:48:31.3590746Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs\__main__.py", line 183, in build_command
2021-06-25T17:48:31.3593441Z     build.build(config.load_config(**kwargs), dirty=not clean)
2021-06-25T17:48:31.3594357Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs\commands\build.py", line 249, in build
2021-06-25T17:48:31.3596750Z     config = config['plugins'].run_event('config', config)
2021-06-25T17:48:31.3597610Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs\plugins.py", line 94, in run_event
2021-06-25T17:48:31.3599597Z     result = method(item, **kwargs)
2021-06-25T17:48:31.3600468Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs_macros\plugin.py", line 502, in on_config
2021-06-25T17:48:31.3635890Z     self._load_modules()
2021-06-25T17:48:31.3636723Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs_macros\plugin.py", line 388, in _load_modules
2021-06-25T17:48:31.3639992Z     self._load_module(module, local_module_name)
2021-06-25T17:48:31.3640849Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs_macros\plugin.py", line 336, in _load_module
2021-06-25T17:48:31.3644133Z     module.define_env(self)
2021-06-25T17:48:31.3644815Z   File "E:\TfsBuildAgents\Default\BuildAgent1\_work\123\s\main.py", line 6, in define_env
2021-06-25T17:48:31.3647619Z     fix_url = env.variables.fix_url # make relative urls point to root
2021-06-25T17:48:31.3648475Z   File "e:\tfsbuildagents\default\buildagent1\_work\123\s\python\tools\lib\site-packages\mkdocs_macros\util.py", line 160, in __getattr__
2021-06-25T17:48:31.3651962Z     raise AttributeError("Cannot find attribute '%s" % name)
2021-06-25T17:48:31.3652601Z AttributeError: Cannot find attribute 'fix_url
2021-06-25T17:48:31.5190836Z ##[error]Cmd.exe exited with code '1'.
2021-06-25T17:48:31.5746153Z ##[section]Finishing: build website
fralau commented 3 years ago

@CrispyDrone Thanks a lot for bringing up this issue, and I understand your point.

This has been a design decision in 0.5.10, as documented in the changelog.

You should now use:

fix_url = env.macros.fix_url # make relative urls point to root

If you want to have code that works in any case you could write something like:

try:
    fix_url = env.macros.fix_url # make relative urls point to root
except AttributeError:
    # mkdocs-macros version < 5.10
    fix_url = env.variables.fix_url # make relative urls point to root
fralau commented 3 years ago

Addition: I don't think I would like to create a fallback in mkdocs-macros as proposed in pull request #92.

For the background, I had thought (intuitively) that treating variables and macros the same was a sound decision, but under jinja2 they are very different entities, which determine different behaviors. Then it created a weird situation, where macros and variables were together, but not filters.

mkdocs-macros has been changed to align with jinja2: now variables, macros, and filters have their own list, which is very clear (better than adding "ifs" and "buts").

I realise that it may break some pre-existing code, but the fix should be easy to make?

CrispyDrone commented 3 years ago

Hello @fralau

Thanks for your prompt replies.

Sure, it's not a big problem as it was easy to fix.

However, I think from the perspective of semantic versioning, this is a breaking change. So in the future, I recommend incrementing the major version.

As a consumer, I like to lock-in to particular major versions so I have the opportunity to adapt to breaking changes at my own leisure. It seems this is possible with pip with pip install mkdocs-macros-plugin==0.* or pip install mkdocs-macros-plugin~=0.0.0. Of course, I don't know how common of a practice this is in the python ecosystem.

Also, I indeed had a look at the changelog, but I must admit I interpreted it differently. When I see deprecated (or even obsolete), I assume I should not use this api any more as it can be removed in a next major version, but I still expect it to work in the version in which it was deprecated or obsoleted. (I see javascript for example denotes an api as obsolete if it has been removed from that version, whereas in dotnet you have an attribute called Obsolete that takes the role of "deprecating" it so people know it might be removed in a next major version. I don't know what approach python takes.)

In any case, feel free to close this issue.

fralau commented 3 years ago

@CrispyDrone Both points are taken:

  1. For the semantic versioning, this should have been a 0.6.0!
  2. It should not have been labeled Deprecated but Removed.

The version numbering cannot be changed any longer, but the changelog has been corrected in c676fa5 .

I will do it better next time 🙂

CrispyDrone commented 3 years ago

@fralau

Thanks.

I do think it should be 1.0.0 (major version increment) though and not 0.6.0 (minor version).