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

Provide a way to opt-in for a whole subdirectory, instead of file by file? #204

Closed
fralau opened this issue Jan 25, 2024 Discussed in #203 · 15 comments
Closed

Provide a way to opt-in for a whole subdirectory, instead of file by file? #204

fralau opened this issue Jan 25, 2024 Discussed in #203 · 15 comments
Assignees
Labels
enhancement New feature or request fixed A fix has been submitted

Comments

@fralau
Copy link
Owner

fralau commented Jan 25, 2024

Discussed in #203

Originally posted by mhussein January 25, 2024
I have few directories in a very large project that uses macros while the rest doesn't. The project is set as opt-in with

render_by_default: false

Is there a way to avoid setting render_macros: true on every single file in those directories that use macros? Is there a way to tell the plugin to run on all files under certain folder(s)?

@fralau
Copy link
Owner Author

fralau commented Jan 25, 2024

Noncommittal: one way to do that, might be to create an mkdocs-macros.yaml file in the directory? 🤔

That would be the place in the code:

        if self.config['render_by_default']:
            # opt-out: force of a page NOT to be interpreted,
            opt_out = ignore_macros == True or render_macros == False
            if opt_out:
                return markdown
        else:
            # opt-in: force a page to be interpreted
            opt_in = render_macros == True or ignore_macros == False
            if not opt_in:
                return markdown

@mhussein
Copy link

Creating a file in the directory is very good. The other approach is to have a setting in the mkdocs yaml file.

There are pros and cons to each approach. I like the self-containment of having a file in the directory, but I know that many prefer to separate settings from docs, so prefer to have all configurations in folders outside the docs folder.

@mhussein
Copy link

something like:

render_by_default: false
render_in_dir: 
    - docs/fancy_docs

@fralau fralau added the enhancement New feature or request label Jan 27, 2024
@fralau
Copy link
Owner Author

fralau commented Jan 30, 2024

We could indeed, do like that.

We already have list parameters, in the config file e.g.

        ('modules', PluginType(list,
                               default=[])),

We could assume that the default directory is the docs directory; though that would change the convention for include_dir(which is the project directory). 🤔

We also assume that this will be recursive, i.e.: for each page processed, make a check whether that page is part of that directory or its subdirectories.

@fralau
Copy link
Owner Author

fralau commented Jan 30, 2024

For the implementation, we could use a function inspired by this (suggested by ChatGPT):

import os

def is_file_in_directory(file_path, directory):
    # Get the absolute path of the directory
    directory = os.path.abspath(directory)

    # Check if the absolute path of the file is a subpath of the directory
    return os.path.abspath(file_path).startswith(directory)

or:

from pathlib import Path

def is_file_in_directory(file_path, directory):
    # Resolve the paths to eliminate any relative parts (like '..')
    directory = Path(directory).resolve()
    file_path = Path(file_path).resolve()

    # Check if the file path is a descendant of the directory
    return directory in file_path.parents

@mhussein
Copy link

We could assume that the default directory is the docs directory; though that would change the convention for include_dir(which is the project directory). 🤔

I don't think we need to, in the example I used below, I had docs as part of the path docs/fancy_docs I think we should keep it that way so we don't change general behavior, unless this will make it harder in the implementation. You will know better.

We also assume that this will be recursive, i.e.: for each page processed, make a check whether that page is part of that directory or its subdirectories.

For this feature, that makes sense. If you are including a directory, then you are including everything under that directory.

Get ready for requests to add a flag for (nonrecursive), or add an option to exclude directory though, it will come sooner or later :)

@fralau
Copy link
Owner Author

fralau commented Feb 17, 2024

I would think the pattern matching syntax, as in .gitignore, but reversed 🤔

For that, we might use the Python PathSpec library.

@fralau fralau added the planned This is planned for correction label Feb 17, 2024
fralau pushed a commit that referenced this issue Feb 19, 2024
  - this is a mechanism that forces rendering of files
    for the case when `render_by_default` is set to `false`.
  - it uses a git-like syntax to define files to include
@fralau fralau added solved The issue has been satisfactorily solved please test An updated was pushed. This needs to be tested and removed planned This is planned for correction labels Feb 19, 2024
@fralau
Copy link
Owner Author

fralau commented Feb 19, 2024

The last commit implements this requirement of opt-in of files by directory, filename, etc when the parameter render_by_default in the config file is set to false.

There is a new parameter called force_render_paths (this is a YAML multiline literal string, note the pipe symbol).

The syntax follows more or less the .gitignore pattern matching: this allows more than one instruction, with examples provided in this page. Comments (starting with a #) are accepted within the string.

Here is an example:

plugins:
  - search
  - macros:
      # do not render the pages by default
      # requires an opt-in
      render_by_default: false
      # render that path:
      force_render_paths: |
        # this directory will be rendered:
        rendered/
        # this pattern of files will be rendered:
        render_*.md

Notes

  1. In the way that it is implemented now, the force_render_paths takes precedence over any other consideration. A page that matches the pattern will always be rendered, even if the header of the page says otherwise ( render_macros is set to false). I haven't made my mind about it yet.

  2. A workin test case can be found on the test/opt-in directory.

  3. I am not sure how much of the git syntax is actually impletemented; in particular, has double star (**) syntax has been implemented. ❓ In any case, I feel it's pretty complete for our needs, so let's declare that it's provided as-is; any comments should be submitted to the authors of the Pathspec library.

@fralau
Copy link
Owner Author

fralau commented Feb 19, 2024

@mhussein Could I ask you to test this commit, and see if it could work for you?

@mhussein
Copy link

mhussein commented Feb 19, 2024

@fralau I tried it and it seems to be working fine for me. Thanks a lot for the quick turnaround.

(1.) I think that if a file has render_macros: false then that should take priority. The local configuration would be expected to override the global one.

btw, I am getting an error with the new version, I think it is unrelated, but there seems to be a conflict with the blogging plugin

[local:internal:run] INFO    -  [macros] - ERROR # _Macro Syntax Error_
[local:internal:run]
[local:internal:run] _File_: `mysite\blogs\index.md`
[local:internal:run]
[local:internal:run] _Line 3 in Markdown file:_ **expected token 'end of print statement', got 'my_blogs'**
[local:internal:run] ```markdown
[local:internal:run] {{ blog_content my_blogs }}

Note: this file has ignore_macros: true in the header, so maybe it is related to (1.)
Update: adding a !mysite/blogs/index.md to the list of paths ignores it, though I still think that local configuration should override global one.

@fralau
Copy link
Owner Author

fralau commented Feb 19, 2024

Thanks a lot. Your argument about local overriding the global makes sense. I will have a look at it.

It's good to know that the negative syntax (with !) also works. 🙂

@fralau
Copy link
Owner Author

fralau commented Feb 19, 2024

Oh, I forgot to specify (but I think you worked it out) that the path is relative to the document directory (docs), not the project's root.

That is how the paths of document pages are referenced by MkDocs, so it was simpler for me to stick to that convention.

It is different than for other parameters, but it makes sense; I will just have to make sure that the documentation is clear.

@mhussein
Copy link

but I think you worked it out

Yes, had to put all possible variations together first tbh, then started eliminating one by one till I figured it out :)

It makes sense though to be consistent in the mkdocs file.

@fralau
Copy link
Owner Author

fralau commented Feb 20, 2024

I updated with cfa03e7, to give the last word to the render_macros parameter in the YAML header of the page (true or false).

I also updated the opt-in test case.

@fralau fralau added fixed A fix has been submitted and removed solved The issue has been satisfactorily solved labels Feb 21, 2024
@fralau fralau self-assigned this Feb 21, 2024
@fralau fralau added the stale No news, closing label Mar 9, 2024
@fralau fralau removed stale No news, closing please test An updated was pushed. This needs to be tested labels Apr 9, 2024
@fralau fralau closed this as completed Apr 9, 2024
@mhussein
Copy link

@fralau Thanks a lot for implementing this. Do you have a plan of when this can be available in a release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed A fix has been submitted
Projects
None yet
Development

No branches or pull requests

2 participants