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.

Sign up for GitHub

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

Tree structure #34

Merged
merged 11 commits into from
Jul 12, 2024
Merged

Tree structure #34

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
feb22b6
port galae website to jssg prototype
Jun 17, 2024
2593e05
Compatibility with jinja2 templates
Jun 20, 2024
0324300
Start of work on tree structure
Jun 25, 2024
b791b94
POC of tree structure for pages
Jul 5, 2024
93a2c7f
Clarify distill fonctions
Jul 8, 2024
0cbb3ba
Distill fonction simplification
Jul 8, 2024
70c4bda
Add url_abs() to allow not unique slugs
Jul 11, 2024
a1f47df
Resolve merge conflicts
Jul 11, 2024
032166c
Merge branch 'main' into feat__12_tree_structure
ClmntBcqt Jul 11, 2024
52888ae
Clarify the code and add docstrings in jinja.py
Jul 12, 2024
8b99f0b
change the slug url format in url_for_slug_path()
Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions content/pages/a_page.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ template_engine jinja2

<h2>Page 1</h2>

<p>Go to <a href="{{ url('page', args=['page2']) }}">page 2</a> </p>

<p>Go to <a href="{{ url_slug('page2') }}">page 2</a> </p>
2 changes: 1 addition & 1 deletion content/pages/another_page.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ template_engine jinja2

<h2> Page 2 </h2>

<p>Go to <a href="{{ url('page', args=['page1']) }}">page 1</a> </p>
<p>Go to <a href="{{ url_slug('page1') }}">page 1</a> </p>
2 changes: 1 addition & 1 deletion content/templates/jinja2/widgets/navbar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<div class="navbar__container__right">
<a class="btn btn-sm btn-primary" href="{{ CTA_URL }}"
target="{{ CTA_TARGET }}">{{ CTA_LABEL }}</a>
<a href="{{ url('page', args=[CHANGE_LANG_URL]) }}">
<a href="{{ url_abs(CHANGE_LANG_URL) }}">
<img class="languageFlag" src="{{ static(CHANGE_LANG_FLAG_URL)}}" alt="{{ CHANGE_LANG_ALT }}"/>
</a>
</div>
Expand Down
36 changes: 34 additions & 2 deletions jssg/jinja2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,47 @@

from jssg.templatetags.filter_opengraph_metadata import filter_opengraph_metadata

from jssg.models import Document
from django.conf import settings

from jssg.models import Page

def url_from_slug(slug) :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
    """

url = ""
pages_with_slug = ""

for page in Page.load_glob(all=True) :
if page.slug == slug :
if pages_with_slug == "" :
url = "/" + page.dir + "/" + page.slug + ".html"
else :
url = ""
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]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the pages_with_slug[:-2]) @ClmntBcqt?

elif url == "" :
raise Exception("url_slug() : slug '%s' not found" % slug)
return url

def url_absolute(url_path) :
dir = '/'.join(url_path.split('/')[:-1])
slug = ''.join((''.join(url_path.split('/')[-1])).split('.')[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used re.findall() so we can check the format of the url given by user too


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 :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

return "/" + dir + "/" + slug + ".html"

raise Exception("url_abs() : page for %s url not found" % url_path)

def environment(**options):
env = Environment(**options)
env.globals.update(
{
"static": static,
"url": reverse,
"url_slug": url_from_slug,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

"url_abs" : url_absolute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the same time, I suggest to rename url_abs to url_for_path() or url_for_slug_path()

}
)
env.filters.update(
Expand Down
37 changes: 24 additions & 13 deletions jssg/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def load(cls, path: Path) -> "Document":

@classmethod
def load_glob(
cls, path: Optional[List[Path]] = None, glob: str = "*.md"
cls, path: Optional[List[Path]] = None, dir = "", glob: str = "*.md", all=False
) -> Iterator["Document"]:
"""Load multiple document.

Expand All @@ -212,7 +212,10 @@ def load_glob(

files = []
for p in path :
files += p.glob(glob)
if all :
files += (p / dir).rglob(glob)
else :
files += (p / dir).glob(glob)
# print(files)
return map(cls.load, files)

Expand All @@ -235,20 +238,28 @@ def __init__(self, content: str, **metadata) -> None:
except KeyError:
self.slug = slugify(self.title)

self.page_dir = self.path
while (self.page_dir not in self.BASE_DIR) :
self.page_dir = self.page_dir.parent

self.dir = str(self.path.relative_to(self.page_dir).parent)
if self.dir == '.' :
self.dir = ''

@classmethod
def load_page_with_slug(cls, slug: str) -> "Page":
return next(filter(lambda p: p.slug == slug, cls.load_glob()))
def load_page_with_slug(cls, slug: str, dir : str) -> "Page":
return next(filter(lambda p: p.slug == slug, cls.load_glob(dir = dir)))

@classmethod
def load_glob(
cls, path: Optional[List[Path]] = None, glob: str = "*.md"
cls, path: Optional[List[Path]] = None, dir = "", glob: str = "*.md", all = False
) -> Iterator["Page"]:
"""Overridden only to make the static typing happy."""
return super().load_glob(path, glob)
return super().load_glob(path, dir, glob, all)

@classmethod
def get_pages(cls):
return ({"slug": p.slug} for p in Page.load_glob())
def get_pages(cls) :
return ({"slug": p.slug} if p.dir == '' else {"dir": p.dir, "slug" : p.slug} for p in Page.load_glob(all = True))


class Post(Page):
Expand All @@ -267,14 +278,14 @@ def __init__(self, content: str, **metadata) -> None:

@classmethod
def load_glob(
cls, path: Optional[List[Path]] = None, glob: str = "*.md"
cls, path: Optional[List[Path]] = None, dir = "", glob: str = "*.md", all = False
) -> Iterator["Post"]:
"""Overridden only to make the static typing happy."""
return super().load_glob(path, glob)

return super().load_glob(path, dir, glob, all)
@classmethod
def get_posts(cls):
return ({"slug": p.slug} for p in Post.load_glob())
def get_posts(cls) :
return ({"slug": p.slug} if p.dir == '' else {"dir": p.dir, "slug" : p.slug} for p in Post.load_glob(all = True))

class Sitemap :
BASE_DIR = settings.JFME_PAGES_DIRS + settings.JFME_POSTS_DIRS
Expand Down
20 changes: 18 additions & 2 deletions jssg/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,24 @@

from jssg import views
from jssg.models import Page, Post
from jssg import settings


# print([p for p in Page.get_pages()])

urlpatterns = [
distill_path(
"", views.IndexView.as_view(), name="index", distill_file="index.html"
),
distill_path(
"pages/<slug:slug>.html",
distill_path("atom.xml", views.PostFeedsView(), name="atom_feed"),
distill_re_path(
r'^(?!posts/)(?P<slug>[a-zA-Z0-9-]+).html$',
views.PageView.as_view(),
name="page",
distill_func=Page.get_pages,
),
distill_re_path(
r'^(?!posts/)(?P<dir>[a-zA-Z-/]+)/(?P<slug>[a-zA-Z0-9-]+).html$',
views.PageView.as_view(),
name="page",
distill_func=Page.get_pages,
Expand All @@ -35,6 +45,12 @@
name="post",
distill_func=Post.get_posts,
),
distill_path(
"posts/<path:dir>/<slug:slug>.html",
views.PostView.as_view(),
name="post",
distill_func=Post.get_posts,
),
distill_path(
"sitemap.xml",
views.SitemapView.as_view(),
Expand Down
7 changes: 5 additions & 2 deletions jssg/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ class PageView(DetailView):
template_name = "page.html"

def get_object(self, queryset=None) -> Model:
print(self.kwargs["slug"])
model = self.model.load_page_with_slug(self.kwargs["slug"])
if "dir" not in self.kwargs.keys() :
self.kwargs["dir"] = ""
print("dir : " + self.kwargs["dir"])
print("slug : " + self.kwargs["slug"])
model = self.model.load_page_with_slug(self.kwargs["slug"], self.kwargs["dir"])
return model


Expand Down
Loading