timvink / mkdocs-charts-plugin

Mkdocs plugin to add plots from data using vegalite
https://timvink.github.io/mkdocs-charts-plugin/
MIT License
84 stars 6 forks source link

Add support for `README.md` as index #11

Closed CieNTi closed 2 years ago

CieNTi commented 2 years ago

First, thanks for this plugin, super simple and it works really well.

Description

Currently it is assumed that the index file is named index.md, and if using README.md the build fails:

DEBUG    -  Building page Markdown-diagrams.md
ERROR    -  Error building page 'Markdown-diagrams.md': 'ChartsPlugin' object has no attribute 'homepage'
Traceback (most recent call last):
  File "/usr/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/usr/lib/python3.9/site-packages/mkdocs/__main__.py", line 187, in build_command
    build.build(config.load_config(**kwargs), dirty=not clean)
  File "/usr/lib/python3.9/site-packages/mkdocs/commands/build.py", line 314, in build
    _build_page(file.page, config, doc_files, nav, env, dirty)
  File "/usr/lib/python3.9/site-packages/mkdocs/commands/build.py", line 220, in _build_page
    output = config['plugins'].run_event(
  File "/usr/lib/python3.9/site-packages/mkdocs/plugins.py", line 102, in run_event
    result = method(item, **kwargs)
  File "/usr/lib/python3.9/site-packages/mkdocs_charts_plugin/plugin.py", line 80, in on_post_page
    return self.add_javascript_variables(output, page, config)
  File "/usr/lib/python3.9/site-packages/mkdocs_charts_plugin/plugin.py", line 105, in add_javascript_variables
    path_to_homepage = self.homepage.url_relative_to(page.file)
AttributeError: 'ChartsPlugin' object has no attribute 'homepage'

It is widely used the one named README.md, and MkDocs supports it:

... Many repository hosting sites provide special treatment for README files by displaying the contents of the README file when browsing the contents of a directory. Therefore, MkDocs will allow you to name your index pages as README.md instead of index.md. In that way, when users are browsing your source code, the repository host can display the index page of that directory as it is a README file. However, when MkDocs renders your site, the file will be renamed to index.html so that the server will serve it as a proper index file.

If both an index.md file and a README.md file are found in the same directory, then the index.md file is used and the README.md file is ignored. ... Source

Proposal 1

I did a fast & dirty successful test, by modifying the following line:

https://github.com/timvink/mkdocs-charts-plugin/blob/8f0c6c08aa85a222c03d2b574178a5632a20c7ef/mkdocs_charts_plugin/plugin.py#L69

into:

        if page.file.src_path == "index.md" or page.file.src_path == "README.md":

It would be possible to you to add support for README.md too? I didn't want to make a full PR for a partial line addition :D

Thanks a lot in advance!

Proposal 2

I do not have deep knowledge of pythonic ways, and I don't like to repeat myself (2 x self.homepage ..), but it seems this could be a simple solution, in where both are supported and preference is for index.md:

    def on_page_content(self, html, page, config, files, **kwargs):
        """
        Store reference to homepage
        """
        if page.file.src_path == "index.md":
            self.homepage = page.file
        elif page.file.src_path == "README.md":
            self.homepage = page.file
timvink commented 2 years ago

Good find, thnx for reporting!

I guess we'll then also have to account for the situation when both are present.

The whole thing is a bit hacky way to find the path to the root of the site, but it works :)

I'll pick this up next time I work on some OS projects :)

CieNTi commented 2 years ago

Thanks for the prompt reply!

I just updated the issue description, by adding what MkDocs says about index vs README precedence, and the updated proposal (I did a wrong intermediate edit, in case you received that notification :D)

timvink commented 2 years ago

I was able to use an attribute named is_homepage instead. Thanks for doing the research for this issue!

Released in https://github.com/timvink/mkdocs-charts-plugin/releases/tag/v0.0.8

CieNTi commented 2 years ago

Much better and elegant :D Thanks!