803399 - Compiler should react to missing variable

Status: RESOLVED DUPLICATE of bug 802845

(L20n :: JS Library, defect, P1)

Description

[Staś Małolepszy :stas]

[GitHub issue by zbraniecki on 2012-10-04T22:19:10Z, https://github.com/l20n/l20n.js/issues/14]
In such a scenario:

    <progress[plural($timeleft)] {
      zero: 'Done',
      almost: 'Almost there!',
      one: '1 minute left',
      many: '{{ $timeleft }} minutes left'
    }>

I believe it would be safe to:
 - return exception because the index cannot be computed
 - in debug mode throw the exception
 - in work env attempt to take the first value out of hash (or the value if it's a string) and see if it'll compute. If yes, return it. If no, throw?

Comment 1

[Staś Małolepszy :stas]

[GitHub comment by stasm on 2012-10-04T22:32:16Z]
How about actually showing "{{ $timeleft }} minutes left" to the user as the value of the string. Is this better than throwing?

Comment 2

[Staś Małolepszy :stas]

[GitHub comment by zbraniecki on 2012-10-04T22:45:24Z]
sounds reasonable. We'd need to store the string in ComplexString then, right?

Comment 3

[Staś Małolepszy :stas]

[GitHub comment by stasm on 2012-10-04T22:54:49Z]
Yes. On parsing.

Comment 4

[Staś Małolepszy :stas]

[GitHub comment by stasm on 2012-10-04T22:58:47Z]
Actually, no.  Hmm.  

    {{ $username }}, you have {{ $unreadCount }} unread messages.

If the developer forgets to pass a value for `username`, but passes `unreadCount=3`, we should probably show something like:

    {{ $username }}, you have 3 unread messages.

This looks fine with `username`, but once we get into more complex symbols, say `app.user.name.first`, and `first` is missing, working our way up in the compiler until we can show `{{ app.user.name.first }}` will be hard.

Comment 5

[Staś Małolepszy :stas]

[GitHub comment by stasm on 2012-10-04T23:09:29Z]
Another solution would be to change our AST.  Right now

    {{ $username }}, you have {{ $unreadCount }} unread messages.

is parsed into: [expression, string, expression, string].  Expressions can be anything.  Here they happen to both be `VariableExpressions`, but for the `{{ app.user.name.first }}` example above, it would be a `PropertyExpression`.

The alternative would be to have a special `Placeable` symbol that can only be found in `ComplexStrings`.  The placeable would hold the expression and the string representation of it, so that the compiler can use it in case of an error.

Comment 6

[Staś Małolepszy :stas]

Gandalf, what do you think about comment 5?  Specifically, about the `Placeable` symbol?  Any other ideas on how to solve this?

I'm going to implement error handling and exception bubbling for this as part of bug 802845.  This will cover the part when we inform the context and the developer that something went wrong.

Let's discuss what to show in lieu of the broken entity here.

Comment 7

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

That would be a problem for us since we don't have, at any point the whole expression. We lex it progressively.

I'd be strongly against trying to change this behavior so we need a way to stringify an expression, which should be fairly easy.

Comment 8

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

It's partially resolved on parser2 branch. It leaves the source string for String element in ComplexString so that we can show it.
It does not allow for partial compilation yet.

Comment 9

[Axel Hecht [:Pike]]

I'm not sure what exactly the needinfo on me is at this point, can I get a multiple-choice question instead? ;-)