Skip to content

Conversation

@fidian
Copy link
Contributor

@fidian fidian commented Jan 23, 2015

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?

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
Copy link
Member

You have far more patience than I :)

How's it do on the mustache spec?

@fidian
Copy link
Contributor Author

fidian commented Jan 23, 2015

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
Copy link
Member

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
Copy link
Contributor Author

fidian commented Jan 23, 2015

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.

@fidian fidian closed this Jan 23, 2015
@bobthecow
Copy link
Member

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

@fidian
Copy link
Contributor Author

fidian commented Jan 23, 2015

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
Copy link
Member

Cool!

@fidian fidian reopened this Jan 26, 2015
@fidian
Copy link
Contributor Author

fidian commented Jan 26, 2015

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
Copy link
Member

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

@bobthecow
Copy link
Member

Looking great, btw!

@fidian
Copy link
Contributor Author

fidian commented Jan 26, 2015

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
Copy link
Member

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

@fidian
Copy link
Contributor Author

fidian commented Jan 26, 2015

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.

  • (D x13) Changing delimiters are not supported
  • (O x12) Representing objects in bash is not supported (bash doesn't do objects)
  • (H x2) HTML encoding is not performed
  • (S x1) Special shell-related issue. In this case it is because the function is using a "global" variable, but it's within a subshell so the variable gets reset.

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)

Conflicts:
	_data/languages.yml
@fidian
Copy link
Contributor Author

fidian commented Feb 19, 2015

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
Copy link
Member

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
Copy link
Contributor Author

fidian commented Feb 19, 2015

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
#54 (comment)
.

@bobthecow
Copy link
Member

Not really.

bobthecow added a commit that referenced this pull request Feb 19, 2015
Adding Bash version of Mustache
@bobthecow bobthecow merged commit 81f9da1 into mustache:master Feb 19, 2015
@fidian
Copy link
Contributor Author

fidian commented Feb 19, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants