Invalid param "not an object" when using {{#tests}} on a non-object

[Niols]

Hello,

I was trying to use Mustache to render a string only when it is non-null. This should essentially be the following template:

Hello
{{#key}}- The key exists: {{key}}{{/key}}
Good bye

It works as expected with the Ruby implementation of Mustache in both cases. It does not however work with the OCaml implementation in the case where key exists. I get Mustache_types.Invalid_param("str. not an object").

My guess is that Mustache is happy with the test {{#key}} and puts the JSON for key in the context. When later rendering {{key}} it fails because the context is not an object. It's actually a bit more than a guess:

In render_fmt, when rendering sections, we have:

ocaml-mustache/lib/mustache.ml

Line 324 in 0333e46

| context -> render' indent s.contents (add_context context js)

where add_context is:

ocaml-mustache/lib/mustache.ml

Lines 268 to 272 in 0333e46

	let add_context ctx js =
	match (ctx, js) with
	| `O ctx_o, `O js_o -> `O (ctx_o @ js_o)
	| _, _ -> ctx
	in

which means that when ctx is not an object, the context becomes this non-object. And when we try to render something afterwards, we have a failure. I would rather have something like:

    let add_context ctx js =
      match (ctx, js) with
      | `O ctx_o, `O js_o -> `O (ctx_o @ js_o)
      | _, `O _ -> js
      | _, _ -> ctx
    in

Does that seem reasonable to you?

[rgrinberg]

Yeah, that seems reasonable to me.

[tsileo]

Hey, is there any way to have the fix in a new opam release?

Should I make a PR (with the fix from @Niols fork)?

Thanks!

[rgrinberg]

Yes, that would be welcome.

[gasche]

I hit this issue independently and proposed a fix in #49. My proposal is more invasive, but it more clearly corresponds to the specification, and there is no regression in existing tests.

[gasche]

(The fix in #49 has been merged, so I think this issue and the other PR #41 could be closed.)