-
Notifications
You must be signed in to change notification settings - Fork 464
Allowing category hierarchies. #1711
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
Conversation
(I tried to make the changes to scan_posts as small as possible to make integrating with the refactor-scanposts branch in #1708 as simple as possible.) |
@@ -11,6 +11,14 @@ | |||
</%block> | |||
|
|||
<%block name="content"> | |||
%if subcategories: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because tag_index.tmpl
lets index.tmpl
do all the work. Usually subcategories
is not set, so nothing will happen.
It would be better to move this into tagindex.tmpl
, but I was too lazy for that during the first try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a special block for this to index.tmpl
for this and use it in tagindex.tmpl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
The invariance check fail in the tests was to be expected, due to the code which creates category hierarchy trees via nested lists. |
Don’t write Jinja templates by hand. Your |
Fixed that. I forgot where to find that script again... Apropos, that script won't work with Python 3. |
# If CATEGORY_ALLOW_HIERARCHIES is set to True, categories can be organized in | ||
# hierarchies. For a post, the whole path in the hierarchy must be specified, | ||
# using a forward slash ('/') to separate paths. Use a backslash ('\') to escape | ||
# a forward slash or a backslash (i.e. '\//\\' is a path specifying the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not enable the insane. It's easier to just use / for separators, and // for a literal /
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then parsing is not unique, since a///b
can be both ['a/', 'b']
and ['a', '/b']
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just parse it left-to-right, like everyone does. a///b
→ ['a/', 'b']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, who even wants a /
in their category name when they have nesting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I am tempted to say that if someone uses a///b as a tag, he deserves the uncertainty ;-)
I just hate how all the clashing slashes look. Maybe get a 3rd opinion? Otherwise, since you are doing the code, you win.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe someone likes to have Unix paths in her/his categories, like /dev/null
. Having that as a subcategory would be impossible if we parse left-to-right. I really don't like the idea of not being able to specify certain (sub-)category names just because we wanted to make things a little simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"web/a//b testing" (posts about a/b testing websites)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have actually seen /dev/null used as a category name :-P
Ok, you win Felix! (shakes fist)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, most people use neither /
nor \
in category names, so essentially nobody will notice this anyway. And the few who really want to use at least one of them, might actually be happy to use what they wanted to use :)
@Kwpolska: how to proceed with the (deliberately) failed invariance tests? |
@felixfontein Ignore them. You can’t do anything about it. We’ll trigger a baseline rebuild when we merge this. |
Out of curiosity: is it something you/we have to do manually, or is it done automatically? |
@felixfontein semi-automatically: just change the build number in the invariant-builds repo after merging the PR (that’s a race condition and we might need to retry the build for this repo sometimes). |
Ok. I'll do some more testing, and if nobody complains until when I'm done, I'll merge it. |
…mparing their slugs.
Fixed error message for unknown escape sequences.
366a0af
to
ae3c911
Compare
Allowing category hierarchies. (fixes #1520)
I cannot commit to the invariant-builds repo, so I cannot trigger a baseline rebuild. @Kwpolska: would you mind doing that? :) |
@@ -15,6 +15,8 @@ New in v7.4.1 | |||
Features | |||
-------- | |||
|
|||
* Allowing category hierarchies via new option CATEGORY_ALLOW_HIERARCHIES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong placement, fixed
This is a first shot on implementing category hierarchies, as discussed in #1520. Hierarchies must be explicitly enabled via
CATEGORY_ALLOW_HIERARCHIES = True
inconf.py
. Then/
in a category name is used to specify a category path, and\
is used for escaping/
and\
.CATEGORY_OUTPUT_FLAT_HIERARCHY
can be used to switch between using the full category path to determine the category's slug (when set toFalse
, which is the default), or to just use the subcategory name for the slug (when set toTrue
).