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.

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

Allow partial overriding of nested YAML content #142

Closed
dcgauld opened this issue Oct 12, 2015 · 22 comments
Closed

Allow partial overriding of nested YAML content #142

dcgauld opened this issue Oct 12, 2015 · 22 comments

Comments

@dcgauld
Copy link

dcgauld commented Oct 12, 2015

A section of an example YAML file:

sections:
  header:
    text@: "Some header text"
  promo:
    heading@: "Heading"

The above YAML is used in the default locale (en_US). If in the en_IN locale I want to override just the promo section, using the code below would override the whole sections object:

---
$locale: en_IN
  sections:
    promo:
      heading@: "New heading"

Obviously the desired outcome is that we only affect the promo subsection.

/cc @meizon

@WillsB3
Copy link

WillsB3 commented Oct 12, 2015

+1 for this :).

@jeremydw
Copy link
Member

Hi! I agree with the premise behind this issue. We definitely need to make sure that the solution for this is "not magical" and that the behavior always meets users' expectations. We right now have an implicit "dictionary update" that occurs when localized documents are used.

I think yaml anchors and aliases (see relevant section at http://rhnh.net/2011/01/31/yaml-tutorial) could be a good start towards adding this feature -- but we'll likely need to do something a little more custom to allow the anchors/aliases to work across documents.

@jeremydw
Copy link
Member

Hi folks -- I have a concept of this implemented. See sample structure below. Please note the $localization: fallback: recursive key that triggers this behavior. I added this flag because I don't think it'd be good to change the current behavior without the user opting-in to another mode.

I like that we've introduced this feature, but I'm not happy with the name of the flag fallback: recursive. Would anyone have any suggestions for a different flag name? Or, would there be any way this behavior/option could be made clearer? Thanks!

As an aside: note that Grow's yaml parser fully supports aliases and anchors -- they just cannot be used across documents per the yaml spec. Attempts to alter this behavior also failed since we use libyaml directly and likely shouldn't modify it.

---
$title: Localized
$localization:
  fallback: recursive
  path: /intl/{locale}/localized/
  default_locale: ja
  locales:
  - ja
  - fr
  - de
nested_items:
  nested_item:
    nested_item_title: "BaseTitle"
    nested_item_content: "BaseContent"
---
$locale: ja
nested_items:
  nested_item:
    nested_item_title: "JaTitle"

@meizon
Copy link

meizon commented Oct 24, 2015

Maybe "replace" as its replacing the contents of that level's key value?

@meizon
Copy link

meizon commented Oct 24, 2015

I also preferred the idea of showing in the localised section that this was going to happen. Can't we opt in to the "replace" on a per locale setting?

@meizon
Copy link

meizon commented Oct 24, 2015

Because you may want to totally replace the value in one locale, but update/merge in another locale?

@jeremydw
Copy link
Member

Hm, thanks for hashing this out with me -- I'm not sure that's good either since it doesn't describe how the replacement behaves. The most descriptive way to describe this behavior is a "recursive update" or a "recursive merge", but that might be too much of a mouthful to use for the config files.

To answer your second question -- yes, can definitely do that. :) We can allow you to specify the replace/update/merge (whatever we're calling it) behavior on a per-locale basis.

@WillsB3
Copy link

WillsB3 commented Oct 24, 2015

I don't suppose this would be configurable on a per-key basis instead of for all keys in the localised document? I assume we can't introduce any special syntax within the key itself because, as you mention, we are using a third-party yaml parser?

I was thinking something like:

---
$title: Localized
$localization:
  path: /intl/{locale}/localized/
  default_locale: ja
  locales:
  - ja
  - fr
  - de
nested_items:
  nested_item:
    nested_item_title: "BaseTitle"
    nested_item_content: "BaseContent"
---
$locale: ja
nested_items+:
  nested_item:
    nested_item_title: "JaTitle"

The "+" is just the first symbol that came into my head for "merge". I put it at the end of the key name to try and be at least a little consistent with how strings are marked for translation with the @ suffix. Note that this also (as far as I can see) negates the need for the fallback option (as you specify the operation on a per key basis).

It's highly likely that I'm missing some obvious flaw in my suggestion, it's late ;).

@jeremydw
Copy link
Member

Yeah -- that's a really nice suggestion actually. Let me iterate on a few variations of that a little.

Believe it or not I'm considering deprecating the @ suffix in favor of a custom yaml constructor for gettext (such as the example below) -- it's a bit more verbose, but this would have the advantage of removing the need to add the gettext function call in templates and it would also speed up yaml loading by a little bit.

old_style_tag@: Hello World
new_style_tag: !_ Hello World

! indicates a yaml constructor, and _ is (of course) the standard alias for gettext.

We could leverage a custom yaml constructor to control fallback behavior as well -- !+ could work just as you've explained. Let me give it a try. I'd rather use custom yaml constructors (since this is basically what they were meant for) rather than renaming keys further!

@meizon
Copy link

meizon commented Oct 25, 2015

How does that work when you need to pass in variables such as _(text, url="url")

@WillsB3
Copy link

WillsB3 commented Oct 25, 2015

Just been thinking about this some more...

What would happen in the case of deeply nested structures? For example I know we've done this sort of thing before:

root_thing:
  sections:
    section_a:
      title: "BaseTitle"
      content: "BaseContent"
    section_b:
      title: "BaseTitle"
      content: "BaseContent"

We've had cases in the past where we've needed to replace the data in section_b but leave section_a (and other data under sections) untouched. Using my previous example this is how I imagine that would have to look:

---
$title: Localized
$localization:
  path: /intl/{locale}/localized/
  default_locale: ja
  locales:
  - ja
  - fr
  - de
root_thing:
  sections:
    section_a:
      title: "BaseTitle"
      content: "BaseContent"
    section_b:
      title: "BaseTitle"
      content: "BaseContent"
---
$locale: ja
root_thing+:
  sections+:
    section_a:
      nested_item_title: "JaTitle"

As you can see this gets complicated (and maybe confusing). In my mind if I wanted to replace section_a for the ja locale I'd have to specify the "merge" modifier (whatever that is, just using my previous syntax here) on every parent property. Would merging child properties of structures like this work or would it only work at the top level?

@WillsB3
Copy link

WillsB3 commented Jan 21, 2016

Hey @jeremydw have you had any more thoughts on how we might achieve this?

@jeremydw
Copy link
Member

jeremydw commented Jun 3, 2016

I finally have something, including a prototype, which I'm very happy with. In this proposal, we no longer leverage multiple YAML documents for localization, but rather a "decorator" for keys in YAML documents that are localized.

Because we keep everything in one document, we can now leverage YAML anchors and aliases for repeating/referencing other content.

Here's an example localized document:

$path: /site/
$localization:
  path: /intl/{locale}/site/

$view: /views/base.html
$view@fr: /views/base_fr.html

$title: Welcome
$title@fr: Welcome in FR
$title@en: Welcome to EN   # Override for "en" only! Allows you to use a different string in root/English without affecting other locales.

hero:
  title: Welcome to our site
  title@de: Welcome to our site (de)

slides:
- title: Root Slide
  title@de: Slide title in DE
  url: http://www.example.com

slides@fr:  # Overrides entire `slides` block for FR.
- title: FR Slide 1
  url: http://www.example.fr
- title: FR Slide 2
  url: http://www.example.fr

oems:
- title: Acer
  url: http://www.acer.com
  url@de: http://www.acer.de
  url@fr: http://www.acer.fr
- title: Motorola
  url: http://www.motorola.com
  url@ru: http://www.motorola.ru

I'm really happy with the above, and I hope that one could deduce the meaning of this content structure just by reading the YAML document. In short, keys that are localized are decorated with @<locale>. If a localized key exists for the document's current locale, Grow uses it. All other localized keys are discarded.

This allows developers to override entire blocks of YAML content (including Grow builtins like $view!) or just specific keys that might be nested deep in some other YAML structure.

Thoughts? I'm working on finishing up some tests and then will have a prototype ready in feature/yaml. The code for this is also quite simple, so I'm pretty happy with it so far.

@meizon
Copy link

meizon commented Jun 3, 2016

Does this mean this will work:

slides:
  - title: Test
    url: http://www.motorola.com
  - title@fr: Test 2
    url@fr: http://www.acer.fr

Means that for all locales will get the first slide but FR will get both slides?

@meizon
Copy link

meizon commented Jun 3, 2016

Also, does this mean that url@fr: http://www.acer.fr will then be included in the localisation catalog which we might not actually want?

@jeremydw
Copy link
Member

jeremydw commented Jun 4, 2016

To answer your first comment:

Based on how the current proposal works, no, in non-FR locales, that list item would simply be empty since Grow would discard the keys ending in @fr (unless if the document were fr, then you'd have two list items).

Off the top of my head, to achieve "use all the root slides, and add a special slide to FR" (which I think is what you're looking to have in your example), you'd have to restructure the data slightly since YAML does not support merging lists with anchors and aliases -- it only supports merging maps (http://yaml.org/type/merge.html, http://www.yaml.org/refcard.html)

Option 1: Create anchors for each slide and enumerate each slide using aliases.

slides:
- &acer
  title: Acer
  url: http://www.acer.com
- &motorola
  title: Motorola
  url: http://www.motorola.com

slides@fr:
- *acer
- *motorola
- title: Asus
  url: http://www.asus.fr

Option 2: Use a mapping instead of a list and use the merge key.

slides: &slides
  acer:
    title: Acer
    url: http://www.acer.com
  motorola:
    title: Motorola
    url: http://www.motorola.com

slides@fr:
  << : *slides
  asus:
    title: Asus
    url: http://www.asus.fr

You might think that option 2 is exactly what you want, as it appears correct. The only issue here is that mappings are unordered, and when this is serialized to JSON and yielded to the template, the order will be non-deterministic. You can fix this by always sorting the data when rendering it inside a template.

For example, frequently you'd want to sort this type of data by something like a date, or alphabetically, anyway -- so hopefully the merge+mapping capability does the trick. As a result, neither option 1 nor option 2 are exactly perfect but my hopes are they're close enough.

To answer your second comment:

No changes to how this works today -- strings that don't end in @ won't be extracted. So if you wanted a localized, and also extracted string, you would do this:

`title@fr@: Hello from FR"

Then the string "Hello from FR" would be extracted.

Also interested in hearing any feedback from @stucox, @WillsB3, and @dcgauld.

@WillsB3
Copy link

WillsB3 commented Jun 4, 2016

I'll very likely have some questions to add here over the next few days 🙂

On Saturday, 4 June 2016, Jeremy Weinstein notifications@github.com wrote:

To answer your first comment:

Based on how the current proposal works, no, in non-FR locales, that list
item would simply be empty since Grow would discard the keys ending in @fr
(unless if the document were fr, then you'd have two list items).

Off the top of my head, to achieve "use all the root slides, and add a
special slide to FR" (which I think is what you're looking to have in your
example), you'd have to restructure the data slightly since YAML does not
support merging lists with anchors and aliases -- it only supports merging
maps (http://yaml.org/type/merge.html, http://www.yaml.org/refcard.html)

Option 1: Create anchors for each slide and enumerate each slide using
aliases.

slides:

slides@fr:

Option 2: Use a mapping instead of a list and use the merge key.

slides: &slides
acer:
title: Acer
url: http://www.acer.com
motorola:
title: Motorola
url: http://www.motorola.com

slides@fr:
<< : *slides
asus:
title: Asus
url: http://www.asus.fr

You might think that option 2 is exactly what you want, as it appears
correct. The only issue here is that mappings are unordered, and when this
is serialized to JSON and yielded to the template, the order will be
non-deterministic. You can fix this by always sorting the data when
rendering it inside a template.

For example, frequently you'd want to sort this type of data by something
like a date, or alphabetically, anyway -- so hopefully the merge+mapping
capability does the trick. As a result, neither option 1 nor option 2 are
exactly perfect but my hopes are they're close enough.

To answer your second comment:

No changes to how this works today -- strings that don't end in @ won't
be extracted. So if you wanted a localized, and also extracted string, you
would do this:

`title@fr@: Hello from FR"

Then the string "Hello from FR" would be extracted.

Also interested in hearing any feedback from @stucox
https://github.com/stucox, @WillsB3 https://github.com/WillsB3, and
@dcgauld https://github.com/dcgauld.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#142 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AA1B4mHVhNB1LoHSF511xZhN-RVZ1jQNks5qIOuWgaJpZM4GNAG0
.

@WillsB3
Copy link

WillsB3 commented Jul 7, 2016

Would this work?

smoothies:
  flavours:
    - name: "Coconut"
      price: 10
    - name: "Strawberry"
      price: 9
  flavours@it:
     << : *smoothies.flavours
     - name: "Amaretti"
       price: 10

My intended result here is that for it locales all 3 flavours are available.

We often have "top level" groupings of things which then contain lists of other things as per this example in our yaml files, and I'm not sure that the merge would work since it's being used within the same "parent" map (and I guess that smoothies would not yet defined until the parser has parsed the entire structure?)

@WillsB3
Copy link

WillsB3 commented Jul 7, 2016

Alternatively, given the above proposal, would this be the way to achieve it?

smoothie_flavours: &smoothie_flavours
  - name: "Coconut"
    price: 10
  - name: "Strawberry"
    price: 9

smoothie_flavours@it: &smoothie_flavours@it
  << : *smoothie_flavours
  - name: "Amaretti"
    price: 10

smoothies:
  ... lots of other things ...
  flavours: *smoothie_flavours
  flavours@it: *smoothie_flavours@it

This seems pretty strange/complex though.

@jeremydw
Copy link
Member

jeremydw commented Jul 8, 2016

The former would work, although you'll need to add an anchor for
&smoothies.flavours before you can use *smoothies.flavours. After merging
in #239 we'll resume working on this to
get it in ASAP.

On Thu, Jul 7, 2016 at 5:23 AM, Wills Bithrey notifications@github.com
wrote:

Would this work?

smoothies:
flavours:
- name: "Coconut"
price: 10
- name: "Strawberry"
price: 9
flavours@it:
<< : *smoothies.flavours
- name: "Amaretti"
price: 10

i.e. We often have "top level" groupings of things which then contain
lists of other things as per this example in our yaml files, and I'm not
sure that the merge would work since it's being used within the same
"parent" map?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#142 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAndfYHPpRLYYe0elug7vfQ4XIB_C11eks5qTO-5gaJpZM4GNAG0
.

jeremydw

@WillsB3
Copy link

WillsB3 commented Jul 8, 2016

That's good news, I honestly didn't expect that to work :)

@jeremydw
Copy link
Member

jeremydw commented Aug 1, 2016

Okay, #239 is merged! Time to resume work on this feature.

jeremydw added a commit that referenced this issue Aug 3, 2016
* Use remap instead of walk for translation untagging. #142
* Implement localized field untagging, add tests covering documents. #142
* Add additional test coverage for lists. #142
* Implement fix for string tagging, handling unordered dictionary. #142
* Add test verifying behavior for tagged, localized strings. #142
* Fixes #142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants