1132038 - Allow values to be empty

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::L10n, defect)

Description

[Staś Małolepszy :stas]

The grammar doesn't currently allow entity values to be empty:

http://l20n.github.io/spec/grammar.html

Should it?  Should we also allow attributes to be empty?  Should we possibly allow this in an explicit way by introducing a None keyword?

(Related to this is bug 816884.)

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm in favor of empty values. I'm against "None" keyword.

Comment 2

[Staś Małolepszy :stas]

I think the summary of this bug is misleading, I'm sorry about that.  I meant to say "allow no values to be specified" as opposed to "allow values that are an empty string".  Zibi, is this how you understood this bug?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yes, but I believe that the l20n has two forms:

<id attr: ""> is:
 - no value
 - attr is ""

<id "" attr: ""> is:
 - value is ""
 - attr is ""

you can't define attr or value to be None and I'd like us to try to avoid having to add it.

That translates to .properties:

`id = ` being:

<id "">

and `id.attr =` being:

<id attr="">

Do you agree?

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch

Sth like this looks like should do the trick

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1132038-l10n-allows-values-to-be-empty > mozilla-b2g:master

Comment 6

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf] from comment #3)

> <id attr: ""> is:
>  - no value
>  - attr is ""

Should this implicit None value also work for entities with no attributes?

  <id>

?

> That translates to .properties:
> 
> `id = ` being:
> 
> <id "">
> 
> and `id.attr =` being:
> 
> <id attr="">
> 
> Do you agree?

Yeah, I think this makes sense.  This means that there is no way to define a None value for an entity in properties.  When we convert roperties to l20n, all empty values will become "" values in l20n.

Comment 7

[Staś Małolepszy :stas]

Comment on attachment 8563076 [details] [diff] [review]
patch

Review of attachment 8563076 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks for adding the tests!

I think we should also update the grammar spec when we're clear on the L20n syntax.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Commit: https://github.com/mozilla-b2g/gaia/commit/c0e7938b28c6e4116c6b4a97e3e5d487d92dd6d6
Merge: https://github.com/mozilla-b2g/gaia/commit/9cfa310aef6d5fc798f1db0ee9dc4e7ef3e7b152

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

L20n.js: https://github.com/l20n/l20n.js/commit/bee30a7b60d6a6ae1ee60c50433e5e2b94295c52

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8563086 [details] [review]
[gaia] zbraniecki:1132038-l10n-allows-values-to-be-empty > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): ---
[User impact] if declined: Devs will have to workaround this for 2.2
[Testing completed]: l20n tests, gaia-try
[Risk to taking this patch] (and alternatives if risky): minimal, it's a one char relaxing change for a regexp
[String changes made]: none

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

2.2: https://github.com/mozilla-b2g/gaia/commit/9ce6d885a56a6e9b25a5ede70a0a3a73963e538f