First shot on #993 for taxonomies, in particular archives. #2778

felixfontein · 2017-05-19T21:29:32Z

I’ve read the guidelines for contributing.
I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
I tested my changes.

This PR adds functionality to the taxonomy system to be able to reference translated classifications in taxonomies, with an example how to use this for archives. See #993.

The generated archives currently look ugly due to no CSS :)

Kwpolska · 2017-05-20T11:48:52Z

nikola/plugins/task/archive.py

@@ -53,6 +53,7 @@ class Archive(Taxonomy):
    minimum_post_count_per_classification_in_overview = 1
    omit_empty_classifications = False
    also_create_classifications_from_other_languages = False
+    other_language_variable_name = 'other_archive_languages'


Update docs (template-variables.rst).

Good point. Thanks for reminding me!

felixfontein · 2017-05-20T22:07:09Z

Ok, basic styles are now there as well.

felixfontein · 2017-05-21T10:25:53Z

nikola/utils.py

+        # translations themselves
+        args = {'translation_manager': self, 'site': site,
+                'posts_per_classification_per_language': posts_per_classification_per_language}
+        signal('_translations_config'.format(basename.lower())).send(args)


Good catch!

felixfontein · 2017-05-21T10:27:40Z

Ok, now there's basic CSS, support for the default themes (base and bootstrap3), and support for sections, tags, categories (with explicit configuration, disabled by default) and authors (enabled by default).

felixfontein · 2017-05-21T10:36:21Z

(Rebasing to master, to get rid of some annoying problems already fixed there.)

Kwpolska

Pretty good, but I’m not a fan of variable variable names and variable variable existence.

Kwpolska · 2017-05-23T18:38:50Z

docs/template-variables.rst

+``posts``                           list<Post>?     List of items for other templates
+``kind``                            str             The classification name
+``permalink``                       str             Permanent link to page
+``<other_language_variable_name>``  list<tuple>     List of triples ``(other_lang, other_classification, title)``


👎 for making even more variable variable names.

How about fixing other_languages then?

Much better (see also: #2778 (comment) )

Kwpolska · 2017-05-23T18:42:33Z

nikola/conf.py.in

+# example if they have different meaning:
+#   [
+#     {'en': 'cloud', 'de': 'Wolke'},  # the fluffy thing in the sky
+#     {'en': 'cloud', 'de': 'Cloud'},  # cloud computing


How is this going to work? Do English-language readers have to separate sky and butt-computing posts themselves, while the Germans get this done for them?

Perhaps we shouldn’t encourage users to make ambiguous layouts like that, and even prevent duplicate values.

That was just the first example which came to my mind :) It could also be the other way around.

The problem is that when you're writing a blog over a longer time, you sooner or later have cases where you use one tag for two different things because the word has multiple meanings. If you start translating stuff, you can end up having that tag split up into two different tags.

While this kind of sucks, I'd really do not want to disallow this. If a user wants to use the same tag for different things (for example, because he has already been doing that for years and only recently noticed), then why not let him do that?

What we can do is not mention this explicitly in a comment, though, if you'd prefer that.

Yeah, don’t mention it. The five users who need it will discover it by mistake, or will fix their tags.

@Kwpolska: so do you want the examples removed/simplified to not use collisions?

Remove lines 360-370, 432-436 and replace them with:

# Format: a list of dicts {language: translation, language2: translation2, …} # See POSTS_SECTION_TRANSLATIONS example above.

Kwpolska · 2017-05-23T18:42:58Z

nikola/conf.py.in

+# If set to True, a tag in a language will be treated as a translation
+# of the literally same tag in all other languages. Enable this if you
+# do not translate tags, for example.
+# TAG_TRANSLATIONS_ADD_DEFAULTS = False


I wouldn’t mind a True value for this with uncommenting.

I thought that the value in the comment should be the default value, so that if you uncomment it nothing will change (at least in this version). Or is that not the case?

It’s a hack: put one value in nikola.py (oh by the way, you have some work to do in this regard) and another in conf.py.in, so new sites get new behavior, and old sites get something safer.

Kwpolska · 2017-05-23T18:51:57Z

nikola/conf.py.in

+# For example:
+#   [
+#     {'en': 'private', 'de': 'Privat'},
+#     {'en': 'business', 'fr': 'Arbeit'},


last time I checked, die Arbeit was German (and is this the same as business?)

LOL, yes, it is. I wanted to look up the French word, but apparently I forgot to do that... Fixed that.

Kwpolska · 2017-05-23T18:54:21Z

nikola/data/themes/base/templates/index.tmpl

@@ -11,10 +11,24 @@
        <link rel="prefetch" href="${posts[0].permalink()}" type="text/html">
    % endif
    ${math.math_styles_ifposts(posts)}
+    %if other_languages is not UNDEFINED and other_languages:


It’d be great if this check was unnecessary, or approached in some different way. (If it’s main index vs taxonomies: pagekind? If it’s taxonomy-dependent: a variable has_other_languages? Define everywhere but make it empty? And of course, you should use True/False, not None/'other_languages')

I've added has_other_languages. This means that now list.tmpl requires has_other_languages to be defined, which might be a problem for custom plugins. It looks like none of the standard plugins (both Nikola core and Nikola plugins repositories) do that except taxonomies.

Kwpolska · 2017-05-23T18:59:11Z

nikola/conf.py.in

+#     {'en': 'private', 'de': 'Privat'},
+#     {'en': 'business', 'fr': 'Arbeit'},
+#   ]
+# SECTION_TRANSLATIONS = []


POST_SECTION_TRANSLATIONS

Fixed. (I also changed the setting below that accordingly.)

Kwpolska · 2017-05-23T19:04:29Z

nikola/utils.py

+        # Step 3: collapse the tree structure into a linear sorted list,
+        # with a node coming before its children.
+
+        def append_node(classifications, node, path=()):


May I request a split of all the hierarchical stuff to a special module? utils.py is quite cluttered, so some special file for those hierarchies will work better. In that case, sort_node, append_node, and perhaps some others, would also be re-usable between functions. (I kinda dislike defining functions inside functions.)

This is an inner function because it works on an ad-hoc defined particular data structure which might never be used anywhere again. Making it publicly available (at module level) means that the data structure needs to be documented and written in stone from now on. That's why I used a function in a function.

About splitting the stuff: sure, that could be done, but it might break backwards compatibility if some plugin uses one of these functions. (Not this one, of course, since it's new, but the other, already existing hierarchical stuff.)

The split should probably cover the following functions/classes, right?

TreeNode

clone_treenode

flatten_tree_structure

sort_classifications

parse_escaped_hierarchical_category_name

join_hierarchical_category_path

Yes. You can maintain the old API by using a from-import in utils.py.

Ok, I'll do that. I think I'll call it hierarchy_utils.py, is that OK for you?

Kwpolska · 2017-05-23T19:05:56Z

nikola/utils.py

+        if site.config.get('{}_TRANSLATIONS_ADD_DEFAULTS'.format(basename), add_defaults_default):
+            self.add_defaults(posts_per_classification_per_language)
+        # Use blinker to inform interested parties (plugins) that they can add
+        # translations themselves


Got any ideas for plugins that would do that?

If someone wants a more complicated scheme (like using Google Translator to automatically translate tag/category/section/... names), they can use this hook to implement that.

IMO it’s unnecessary.

I don't like preventing extensibility if it is so simple and easy to achieve. (And also very efficient. This is only called once per plugin initialization.)

Kwpolska · 2017-05-23T19:10:07Z

nikola/data/themes/base/assets/css/theme.css

 	display: none;
 }

+.translationslist p:before {


You can add that to the existing .metadata p:before.

Kwpolska · 2017-05-24T13:58:43Z

nikola/conf.py.in

+# For example:
+#   [
+#     {'en': 'car', 'de': 'Auto'},
+#     {'en': 'window', 'fr': 'fenêtre'},


I’d prefer to see {'en': 'window', 'de': 'Fenster', 'fr': 'fenêtre'} to demonstrate the fact that those dictionaries can contain multiple languages.

Since that example is now gone, I added {'en': 'work', 'fr': 'travail', 'de': 'Arbeit'}, in the post sections example.

Kwpolska · 2017-05-24T14:00:49Z

nikola/conf.py.in

+# example if they have different meaning:
+#   [
+#     {'en': 'cloud', 'de': 'Wolke'},  # the fluffy thing in the sky
+#     {'en': 'cloud', 'de': 'Cloud'},  # cloud computing


Yeah, don’t mention it. The five users who need it will discover it by mistake, or will fix their tags.

Kwpolska · 2017-05-24T14:05:46Z

nikola/conf.py.in

+# For example:
+#   [
+#     {'en': 'private', 'de': 'Privat'},
+#     {'en': 'business', 'fr': 'travail'},


'en': 'work'

also, I would not duplicate examples — just make one for sections and in other cases, say “refer to post_sections_whatever documentation”.

Kwpolska · 2017-05-24T18:53:02Z

nikola/utils.py

+        # Step 3: collapse the tree structure into a linear sorted list,
+        # with a node coming before its children.
+
+        def append_node(classifications, node, path=()):


Kwpolska · 2017-05-24T18:58:55Z

nikola/conf.py.in

+# example if they have different meaning:
+#   [
+#     {'en': 'cloud', 'de': 'Wolke'},  # the fluffy thing in the sky
+#     {'en': 'cloud', 'de': 'Cloud'},  # cloud computing


Remove lines 360-370, 432-436 and replace them with:

# Format: a list of dicts {language: translation, language2: translation2, …} # See POSTS_SECTION_TRANSLATIONS example above.

Kwpolska · 2017-05-25T11:40:01Z

nikola/plugins/task/authors.py

@@ -138,6 +140,10 @@ def provide_context_and_uptodate(self, author, lang, node=None):
        kw.update(context)
        return context, kw

+    def get_other_language_variants(self, author, lang, classifications_per_language):


(Codacy’s Parameters differ from overridden 'get_other_language_variants' method suggestion here and in other places is pretty sound)

Should be fixed now.

(Now even a bit more ;) )

felixfontein · 2017-06-01T20:59:53Z

@Kwpolska: I think everything you mentioned is fixed.

felixfontein · 2017-06-04T18:27:46Z

(I rebased to get rid of several merge conflicts.)

…rue for taxonomy.

…lations. Unifying some variables for taxonomy pages.

Kwpolska · 2017-06-10T16:17:47Z

translation_link_helper is a bad name for this. Make it feed_translation_helper.tmpl with functions renamed eg. feed_head.

felixfontein · 2017-06-10T16:50:26Z

If the helper would only be for feeds, I'd agree. But the whole point of the rename is that it has functions not only for feeds, but also for links to translated HTML pages!

felixfontein · 2017-06-10T16:51:32Z

(link should be renamed to feed_link, though, since that's the only function dealing only with feeds.)

Kwpolska · 2017-06-10T16:52:16Z

feed_ should be included in both the filename and macro names.

felixfontein · 2017-06-10T16:52:49Z

Why should it be in the file name? That makes no sense at all.

Kwpolska · 2017-06-10T16:53:49Z

83% of the file is concerned with feeds, not translations.

felixfontein · 2017-06-10T16:55:29Z

I counted. 50 lines are related to translations, 39 lines are related to feeds for blogs with only one language.

felixfontein · 2017-06-10T16:57:03Z

The template generates links to other representations. These can be feeds, translations, or both.

Signed-off-by: Chris Warrick <kwpolska@gmail.com>

felixfontein · 2017-06-10T17:02:28Z

I strongly advise against the new name.

felixfontein · 2017-06-10T17:15:40Z

That should fix the tests.

Kwpolska · 2017-06-11T16:54:23Z

Thanks for doing this!

felixfontein · 2017-06-11T21:06:57Z

Thanks for reviews and merging :)

gwax · 2017-06-12T00:42:08Z

@felixfontein thanks for doing this; it's an important thing to get right.

felixfontein added the in progress label May 19, 2017

Kwpolska reviewed May 20, 2017

View reviewed changes

felixfontein commented May 21, 2017

View reviewed changes

felixfontein force-pushed the taxonomy-lang-refs branch from f51d467 to a4d31c7 Compare May 21, 2017 10:35

felixfontein added enhancement i18n and removed in progress labels May 21, 2017

felixfontein mentioned this pull request May 21, 2017

hreflang rel=alternate is not implemented for galleries and tags #993

Closed

Kwpolska requested changes May 23, 2017

View reviewed changes

Kwpolska reviewed May 23, 2017

View reviewed changes

Kwpolska reviewed May 25, 2017

View reviewed changes

felixfontein force-pushed the taxonomy-lang-refs branch from a4c0a4a to 884a488 Compare June 4, 2017 18:27

felixfontein added 12 commits June 6, 2017 20:53

First shot on #993 for taxonomies, in particular archives.

3858746

Making codacy happier.

23c8000

Adding documentation.

6851d9f

Oops.

786efbb

Adding styles.

b5dc563

Moved sort_classifications function into utils.

9bb3e12

Forgot generic documentation.

f5b6e52

Fixing bug when also_create_classifications_from_other_languages is T…

cbc61c7

…rue for taxonomy.

Adding tag/category/section translations. Adding default author trans…

815eecd

…lations. Unifying some variables for taxonomy pages.

Forgot formatting indicator.

6fd118d

Forgot one change.

200c2b6

Removed unneeded variable.

ba1419b

Renaming link to feed_link.

7439dab

getnikola deleted a comment Jun 10, 2017

Rename translation_link_helper.tmpl → feeds_translations_helper.tmpl

e1d0235

Signed-off-by: Chris Warrick <kwpolska@gmail.com>

getnikola deleted a comment Jun 10, 2017

Fixed names.

e17cb8c

getnikola deleted a comment Jun 10, 2017

Avoid creating broken links.

15e9a9a

getnikola deleted a comment Jun 11, 2017

Kwpolska added this to the v7.8.8 milestone Jun 11, 2017

Kwpolska self-assigned this Jun 11, 2017

Kwpolska added the BLOCKER label Jun 11, 2017

getnikola deleted a comment from felixfontein Jun 11, 2017

Kwpolska approved these changes Jun 11, 2017

View reviewed changes

Kwpolska merged commit afe6799 into master Jun 11, 2017

Kwpolska deleted the taxonomy-lang-refs branch June 11, 2017 16:54

felixfontein mentioned this pull request Jun 16, 2017

Taxonomy language link left-overs #2844

Merged

First shot on #993 for taxonomies, in particular archives. #2778

First shot on #993 for taxonomies, in particular archives. #2778

Conversation

felixfontein commented May 19, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixfontein commented May 20, 2017

Choose a reason for hiding this comment

felixfontein commented May 21, 2017

felixfontein commented May 21, 2017

Kwpolska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixfontein commented Jun 1, 2017

felixfontein commented Jun 4, 2017

Kwpolska commented Jun 10, 2017

felixfontein commented Jun 10, 2017

felixfontein commented Jun 10, 2017

Kwpolska commented Jun 10, 2017

felixfontein commented Jun 10, 2017

Kwpolska commented Jun 10, 2017

felixfontein commented Jun 10, 2017

felixfontein commented Jun 10, 2017

felixfontein commented Jun 10, 2017 • edited

felixfontein commented Jun 10, 2017

Kwpolska commented Jun 11, 2017

felixfontein commented Jun 11, 2017

gwax commented Jun 12, 2017

felixfontein commented May 19, 2017 •

edited

felixfontein commented Jun 10, 2017 •

edited