[CLA] [patch] dojo.substitute() returns undefined for unknown place holders

[wolfram]

Currently dojo.string.substitute works like this:

    >>> dojo.string.substitute("${i} is not defined, but ${x} is", {x:"'this'"});
    undefined

but it should do this imho

    >>> dojo.string.substitute("${i} is not defined, but ${x} is", {x:"'this'"});
    ${i} is not defined, but 'this' is

My usecase: I am doing replacement in multiple steps and want to replace only when the right data are given.

[Adam Peller]

this conflicts with the stated behavior that dojo.string.substitutes throws if a match fails. This was done deliberately to flag cases where strings and code are mismatched that might otherwise go unnoticed by the developer. We can revisit that decision, but I don't think we can have it both ways (we don't want another flag here, do we?)

[wolfram]

what do you mean by "both ways" does that not require a flag? Using the patch i provided would make it at least work like the String.replace method works ... just another thought

[wolfram]

and its currently not throwing an exception or anything, it just returns undefined, which imho feels a bit odd

[Adam Peller]

it throws for me, in trunk at least. What browser are you on?

dojo.string.substitute("${i} is not defined, but ${x} is", {x:"'this'"});

TypeError?: value has no properties

If a missing param doesn't throw, it's currently a bug.

The documented behavior was intentional, and different from the generic string.replace. The theory was that most of the time, it's a one-shot deal, and it's a very common error to miss a substitution or misspell something, so it's useful to flag a mismatch of data and code (IMO)

In your use case, is it possible/reasonable to accumulate the hash using a mixin or beget pattern and process it when you're ready?

[wolfram]

This patch adds tests for dojo.string.substitue() that show the current misbehaviour

[wolfram]

Replying to peller:

it throws for me, in trunk at least. What browser are you on?

dojo.string.substitute("${i} is not defined, but ${x} is", {x:"'this'"});

TypeError?: value has no properties

If a missing param doesn't throw, it's currently a bug.

The documented behavior was intentional, and different from the generic string.replace. The theory was that most of the time, it's a one-shot deal, and it's a very common error to miss a substitution or misspell something, so it's useful to flag a mismatch of data and code (IMO)

In your use case, is it possible/reasonable to accumulate the hash using a mixin or beget pattern and process it when you're ready?

try the patch i just added, it fails for me. and the fault is the two additional test cases i added. can you confirm that?

[Adam Peller]

You're right, there should be a test case for this. I will incorporate your patch, thanks.

The assertError case works fine. However, if you call assertFalse on code that throws an exception, the exception will throw through the assert and doh will will report an error. In other words, assertFalse assumes a false result without an error, so I don't think the first test case is valid.

[Adam Peller]

(In [11128]) Test for unmatched substitute. Refs #4763

[liucougar]

In [28601]:

fixes #4763: modified various widgets to use dojo/mouse.wheel

[bill]

Actually above checkin was for #3763.