-
Notifications
You must be signed in to change notification settings - Fork 295
Adding Bash version of Mustache #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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?
|
You have far more patience than I :) How's it do on the mustache spec? |
|
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. |
|
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 |
|
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. |
|
For now, add it here and note the exceptions like |
|
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! |
|
Cool! |
|
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 I think I might need to work on the line endings for closing standalone tags, but several of those tests are currently passing. |
|
I had to go check to see how you're passing lambda tests :) |
|
Looking great, btw! |
|
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. :-) |
|
Yeah, I did find it. I thought the perl fallback was clever :) |
|
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 |
Conflicts: _data/languages.yml
|
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? |
|
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. |
|
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
|
|
Not really. |
Adding Bash version of Mustache
|
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. |
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?