mustache / mustache.github.com

The {{official}} website
http://mustache.github.io/
Other
2.31k stars 293 forks source link

Adding Bash version of Mustache #54

Closed fidian closed 9 years ago

fidian commented 9 years ago

If it's not worthy of inclusion, would you assist me and file some issues on the project so I know what more could be done?

bobthecow commented 9 years ago

You have far more patience than I :)

How's it do on the mustache spec?

fidian commented 9 years ago

I didn't realize there was a mustache spec. It looks like there's a lot of things mustache does that aren't covered well (or that I missed) in the man pages. For instance, I remove everything between "{{!" and "}}" but didn't know that non-newline characters before and a following newline character after should get removed. I do handle a lot, such as iteration, passing contents to a function, calling functions (when used as a value), triple-braced values, negative match, allowing for when the value is "falsy" (I've defined that as an empty string or unset as there's no real "false" in bash).

There's a few things that bash doesn't support well, such as objects. I also did not auto-escape HTML characters as I suspected this would be used to generate config files and the like. What are the requirements for being listed as a supported language? If I'm nowhere near that mark, I shall close this pull request and wait until I am ready and meet enough of the criteria.

bobthecow commented 9 years ago

There are no formal requirements for being listed, I was mostly curious as to the level of support.

Empty string and unset are great falsey values for Bash. I'd be inclined to give Bash a pass on things like html escaping as well, because that only makes sense in an HTML context, which Bash almost never is :)

There's a lot of variance in what makes sense to support in any given language. I think most of us strive to implement the full spec, or to carve out explicit exceptions, but there's no hard rule about any of it.

The "remove lines containing standalone tags" thing is even a bit unclear in the spec, IIRC. Basically, if a delimiter change, section, inverted section, end section, partial, or comment tag is on a line that matches /^\s*{{...}}\n$/, the whole line should be removed.

fidian commented 9 years ago

I'll close this pull request for now until I have something that runs specs and maybe I'll clear up some of those empty lines as well.

bobthecow commented 9 years ago

For now, add it here and note the exceptions like mustachejs or php-mustache does.

fidian commented 9 years ago

Because you were curious, I've made a test runner, added support for & and now I fail 62 out of 122 tests. I'm sure some shouldn't have passed but they do because of oddities in the tests. For instance, I don't yet remove whitespace and "Surrounding Whitespace" and "Outlying Whitespace (Inline)" pass. Additionally, I don't use dotted notations and "Dotted Names - Broken Chains" passes just fine. :-)

Maybe in the next few days I'll resubmit this change, but only after I look at each of those 62 failing tests. Thanks again for telling me about the spec!

bobthecow commented 9 years ago

Cool!

fidian commented 9 years ago

I've ran the tests and below are the specs that are failing. Almost all of them are due to either delimiters changing (which I don't support) or objects (eg {{a.b.c}} syntax). A couple are due to the HTML escaping (another feature I don't have).

I think I might need to work on the line endings for closing standalone tags, but several of those tests are currently passing.

Summary
=======

* Comments (failed 0 out of 11 tests)
* Delimiters (failed 11 out of 14 tests)
    * FAIL - Pair Behavior
    * FAIL - Special Characters
    * FAIL - Sections
    * FAIL - Inverted Sections
    * FAIL - Partial Inheritence
    * FAIL - Post-Partial Behavior
    * FAIL - Standalone Tag
    * FAIL - Indented Standalone Tag
    * FAIL - Standalone Line Endings
    * FAIL - Standalone Without Previous Line
    * FAIL - Standalone Without Newline
* Interpolation (failed 6 out of 31 tests)
    * FAIL - HTML Escaping
    * FAIL - Dotted Names - Basic Interpolation
    * FAIL - Dotted Names - Triple Mustache Interpolation
    * FAIL - Dotted Names - Ampersand Interpolation
    * FAIL - Dotted Names - Arbitrary Depth
    * FAIL - Dotted Names - Initial Resolution
* Inverted (failed 3 out of 21 tests)
    * FAIL - Context
    * FAIL - Dotted Names - Truthy
    * FAIL - Standalone Line Endings
* Lambdas (failed 4 out of 10 tests)
    * FAIL - Interpolation - Alternate Delimiters
    * FAIL - Interpolation - Multiple Calls
    * FAIL - Escaping
    * FAIL - Section - Alternate Delimiters
* Partials (failed 5 out of 11 tests)
    * FAIL - Recursion
    * FAIL - Standalone Line Endings
    * FAIL - Standalone Without Previous Line
    * FAIL - Standalone Without Newline
    * FAIL - Standalone Indentation
* Sections (failed 5 out of 25 tests)
    * FAIL - Context
    * FAIL - Deeply Nested Contexts
    * FAIL - List
    * FAIL - Dotted Names - Truthy
    * FAIL - Standalone Line Endings

Failed 34 out of 123 tests
bobthecow commented 9 years ago

I had to go check to see how you're passing lambda tests :)

bobthecow commented 9 years ago

Looking great, btw!

fidian commented 9 years ago

In case you didn't see the bit in the run-spec.js that made the lambda tests, I can explain it here. First, I have a "bash" section of code that I've submitted to the specs repository, but my code falls back to the perl version if the bash one isn't available. Next, when writing the test scenario to files for execution, it generates the lambda:

lambda() { echo "__${1}__" }

or

lambda() { perl -e 'print ((PERL_CODE_HERE)->("$1"));'; }

I had lambda support when I first submitted the project and before I knew of the official specs. :-)

bobthecow commented 9 years ago

Yeah, I did find it. I thought the perl fallback was clever :)

fidian commented 9 years ago

I've gone through all 28 of the failing tests. The reasons for the test failures are listed here. I've even tagged them with a letter and provided the number of times that problem showed up.

If you run against the official specs (without mustache/spec#86 merged in) then you may get a couple more issues as I haven't extensively executed the perl fallback options.

I believe this is about at the limit of what pure bash can do without resorting to external tools, such as jq for parsing json.

* Comments (failed 0 out of 11 tests)
* Delimiters (failed 11 out of 14 tests)
    * FAIL - Pair Behavior (D)
    * FAIL - Special Characters (D)
    * FAIL - Sections (D)
    * FAIL - Inverted Sections (D)
    * FAIL - Partial Inheritence (D)
    * FAIL - Post-Partial Behavior (D)
    * FAIL - Standalone Tag (D)
    * FAIL - Indented Standalone Tag (D)
    * FAIL - Standalone Line Endings (D)
    * FAIL - Standalone Without Previous Line (D)
    * FAIL - Standalone Without Newline (D)
* Interpolation (failed 6 out of 31 tests)
    * FAIL - HTML Escaping (H)
    * FAIL - Dotted Names - Basic Interpolation (O)
    * FAIL - Dotted Names - Triple Mustache Interpolation (O)
    * FAIL - Dotted Names - Ampersand Interpolation (O)
    * FAIL - Dotted Names - Arbitrary Depth (O)
    * FAIL - Dotted Names - Initial Resolution (O)
* Inverted (failed 2 out of 21 tests)
    * FAIL - Context (O)
    * FAIL - Dotted Names - Truthy (O)
* Lambdas (failed 4 out of 10 tests)
    * FAIL - Interpolation - Alternate Delimiters (D)
    * FAIL - Interpolation - Multiple Calls (S)
    * FAIL - Escaping (H)
    * FAIL - Section - Alternate Delimiters (D)
* Partials (failed 1 out of 11 tests)
    * FAIL - Recursion (O)
* Sections (failed 4 out of 25 tests)
    * FAIL - Context (O)
    * FAIL - Deeply Nested Contexts (O)
    * FAIL - List (O)
    * FAIL - Dotted Names - Truthy (O)
fidian commented 9 years ago

I just updated my patch to add Bash. The merge conflict that this issue was reporting was because another language was added to the list, yet Bash is getting no love.

Is there a reason this patch is being ignored or shunned? Or is it that the right people haven't had time to sign off on this pull request?

If this shouldn't get merged for some reason, I would love to discuss it. I'm having difficulty understanding why a younger patch to add Swift was merged and this pull request has been sitting dormant for 3+ weeks. Is there something I need to do or a concern that needs to be addressed?

bobthecow commented 9 years ago

It's not that this pull request is lacking in any way, it's that we dropped the ball. Sorry about that.

This should be above the swift implementation in the list though, so if you want to make that change in the PR, I'll merge it.

fidian commented 9 years ago

Does order matter that much? I don't really care where I am listed.

Tyler Akins

On Thu, Feb 19, 2015 at 3:12 PM, Justin Hileman notifications@github.com wrote:

It's not that this pull request is lacking in any way, it's that we dropped the ball. Sorry about that.

This should be above the swift implementation in the list though, so if you want to make that change in the PR, I'll merge it.

— Reply to this email directly or view it on GitHub https://github.com/mustache/mustache.github.com/pull/54#issuecomment-75067980 .

bobthecow commented 9 years ago

Not really.

fidian commented 9 years ago

Awesome, thanks!

I had thought that this pull request might have slipped through the cracks but couldn't be sure. I'm happy that I didn't start with "Hey you mean people, how come the world is out to get me!!?!" or nonsense like that.

I appreciate you taking the time earlier to tell me about the mustache spec and later when you poked around to see how I did the lambda tests.