Tree structure #34
Tree structure #34
Conversation
Add macros for jinja2 widget templates
Add tree structure urls for posts
Management of exceptions to detect dead or ambiguous links
jssg/jinja2.py
Outdated
|
|
||
| def environment(**options): | ||
| env = Environment(**options) | ||
| env.globals.update( | ||
| { | ||
| "static": static, | ||
| "url": reverse, | ||
| "url_slug": url_from_slug, |
There was a problem hiding this comment.
I think naming url_for_slug() this will make templates easier to understand: the usage will be more clear:
<p>Go to <a href="{{ url_for_slug('page1') }}">page 1</a> </p>
jssg/jinja2.py
Outdated
|
|
||
| def environment(**options): | ||
| env = Environment(**options) | ||
| env.globals.update( | ||
| { | ||
| "static": static, | ||
| "url": reverse, | ||
| "url_slug": url_from_slug, | ||
| "url_abs" : url_absolute |
There was a problem hiding this comment.
At the same time, I suggest to rename url_abs to url_for_path() or url_for_slug_path()
jssg/jinja2.py
Outdated
|
|
||
| from jssg.models import Page | ||
|
|
||
| def url_from_slug(slug) : |
There was a problem hiding this comment.
You should add typing + some explanation:
def url_from_slug(slug: str) -> str:
"""
example of slug: xxx
example of result: xxx
raises an exception if the slug is not unique (eg same slug found in several folders)
"""
jssg/jinja2.py
Outdated
| pages_with_slug += str(page.path.relative_to(page.page_dir.parent)) + ", " | ||
|
|
||
| if url == "" and pages_with_slug != "" : | ||
| raise Exception("url_slug() : slug '%s' is not unique, found in : [%s] ; use url_abs()" % (slug, pages_with_slug[:-2])) |
There was a problem hiding this comment.
You should return the list of paths where the slug is found. Eg: /fr/produits/tracim.html, /en/products/tracim.html so that the user knows where the conflict comes from. (I'm not sure, but I think you return only one path?)
By the way, the coed pages_with_slug[:-2]) is not clear at all. What is the :-2 (I mean, even if I know python, I don't know what data is to be found in this selector)
There was a problem hiding this comment.
We can't directly return a list since it is the return value that is printed in the jinja2 template.
But if the slug is not unique, an exception is raised, and all the pages that have this slugs are printed in the error message to inform the user.
There was a problem hiding this comment.
What about the pages_with_slug[:-2]) @ClmntBcqt?
jssg/jinja2.py
Outdated
|
|
||
| def url_absolute(url_path) : | ||
| dir = '/'.join(url_path.split('/')[:-1]) | ||
| slug = ''.join((''.join(url_path.split('/')[-1])).split('.')[0]) |
There was a problem hiding this comment.
the code is quite complex. It looks like the slug is the filename part without extension? If so, look at https://stackoverflow.com/questions/678236/how-do-i-get-the-filename-without-the-extension-from-a-path-in-python for an easy-to-read solution.
What about https://note.nkmk.me/en/python-os-basename-dirname-split-splitext/#get-the-directory-folder-name-from-a-path-ospathdirname for the dir part? Is this what you implement?
This kind of code should at least come with an example. Something like Example: if url_path is "/tmp/folder/subfolder/thefile.html", then slug will be "thefile" and the dir will be "/tmp/folder/subfolder"
Examples are extremly usefull in this context because there are always questions about "absolute or relative ?" "traling slash or no trailing slash?" etc.
There was a problem hiding this comment.
I have used re.findall() so we can check the format of the url given by user too
jssg/jinja2.py
Outdated
| for page in Page.get_pages() : | ||
| if page["slug"] == slug and dir == "" : | ||
| return "/" + slug + ".html" | ||
| elif page["slug"] == slug and "dir" in page.keys() and page["dir"] == dir : |
There was a problem hiding this comment.
what is the condition about "dir" being in the keys? Is that a metadata? something else? Does this mean that a page (Document?) object as an attribute named dir? If so, I suggest to give a more explicity name. Is it a parent folder? a folder path? A parent folder path?
The more explicit the name is, the more comprehensible it is for future maintainers.
There was a problem hiding this comment.
I renamed dir to rel_folder_path.
I changed this part to iterate into Page.load_glob() instead, like this we have access to attributes slug and rel_folder_path (the page folder path relative to its content pages directory)
from /folder/test/file.html to /folder/test/file
PR for issue #12