feat: ✨ Add an option to remove module prefix #199
Conversation
This add an `add_module_prefix` option that is on by default to respect the original behavior.
|
Before: After: |
|
Hmmm it actually breaks on the titles of pages that contain multiple submodules (member links still works) Edit: |
| title = self._escape(obj.name) | ||
| if not self.add_module_prefix and isinstance(obj, docspec.Module): | ||
| object_id = ".".join(object_id.split('.')[1:]) | ||
| title = ".".join(title.split('.')[1:]) |
There was a problem hiding this comment.
I think you want to take the last element after the split, not "all but the first", right?
What you have no gives
my.module -> module
my.module.utils -> module.utils
but I feel like the prefix in the second example is not my but my.module.
There was a problem hiding this comment.
I will test it now with different configuration settings!
for my case, it's what I want but let me check thoroughly.
There is no tests right?
There was a problem hiding this comment.
Currently reworking the unit tests, which should make it easier for people to create new tests. If you merge develop into your branch, you can then add a new .txt file into the pydoc-markdown/src/test/testcases/renderers/markdown folder. Examples are here: https://github.com/NiklasRosenstein/pydoc-markdown/tree/develop/pydoc-markdown/src/test/testcases/renderers/markdown
We should definitely have a unit test for this option. :)
There was a problem hiding this comment.
but I feel like the prefix in the second example is not
mybutmy.module.
I see what you meant now! I'm not sure of the best approach!
There was a problem hiding this comment.
Using ".".join(title.split('.')[1:])
Using title.split('.')[-1]
I'm not sure of what is more expected from the option! Your call 😋
There was a problem hiding this comment.
wait shouldn't it be instead (indentation important):
- helpers
- enumItemsFromList
- addons
- addon_check
- append
- ...
- ...
rather than the highly misleading:
- helpers
- enumItemsFromList
- addons
- addon_check
- append
- ...
- ...
There was a problem hiding this comment.
Yes, you are right, but indentation should be there only when all prefixes are removed right?
Because with the default settings it will be rather strange:
- mtb_blender.helpers
- mtb_blender.helpers.addons
- addon_check
- mtb_blender.helpers.addons
edit: Maybe not, what do you think?
There was a problem hiding this comment.
Good point @casperdcl , this is a bit confusing when documenting multiple modules in the same file and disabling the new add_module_prefix option. Currently in docspec (which pydoc-markdown uses), modules are handled non-recursively (so you get a flat list of module objects with fully qualified module names rather than a nested tree that includes modules), that's one reason where this comes from.
I was planning to add an option to docspec to keep the module hierarchy in tact, but will have to do it in a backwards compatible way. That would make the toc here render with the proper indentations (and the split "hack" from this PR would not be needed), but it will have some implications for how the processors/renderers work in pydoc-markdown.
There was a problem hiding this comment.
well I think either "option to remove module prefix" should also automatically "add indentation too"... or we also need to provide a separate "option to indent"
There was a problem hiding this comment.
The current state is still valid if you're only including a single module into one page. If you're willing to invest into the changes needed to fix the TOC indentation (I'd prefer to go with another option here, it could default to being enabled when add_module_prefix is turned off), I'm happy to review another PR. Otherwise, I'll keep it on my list for when docspec supports module hierarchy.
pydoc-markdown/src/pydoc_markdown/contrib/renderers/markdown.py
Outdated
Show resolved
Hide resolved
Not entirely sure I can follow. Can you explain more? |
As discussed in #198, this add an
add_module_prefixoption that is on by default to respectthe original behavior.