Skip to content
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

Add support for Liquid #393

Open
hughker opened this issue Jun 10, 2015 · 42 comments
Open

Add support for Liquid #393

hughker opened this issue Jun 10, 2015 · 42 comments

Comments

@hughker
Copy link

hughker commented Jun 10, 2015

Would love to see support for Liquid if possible: https://github.com/Shopify/liquid

@Glavin001
Copy link
Owner

Here's a Liquid example:

<ul id="products">
  {% for product in products %}
    <li>
      <h2>{{ product.name }}</h2>
      Only {{ product.price | price }}

      {{ product.description | prettyprint | paragraph }}
    </li>
  {% endfor %}
</ul>

It looks like ERB.

Using http://prettydiff.com/ (/cc @prettydiff) I think it works right now:

Input:

<ul id="products">
{% for product in products %}
<li>
<h2>{{ product.name }}</h2>
Only {{ product.price | price }}
{{ product.description | prettyprint | paragraph }}
</li>
{% endfor %}
</ul>

Output:

<ul id="products">
    {% for product in products %}
    <li>
        <h2>{{ product.name }}</h2>
        Only {{ product.price | price }} {{ product.description | prettyprint | paragraph
            }}
    </li>
    {% endfor %}
</ul>

@hughker please play around with Pretty Diff's beautification at http://prettydiff.com/ and let me know if that does work. If so, then this could be added very soon.

@Glavin001 Glavin001 added this to the v0.30.0 milestone Jun 10, 2015
@Glavin001 Glavin001 self-assigned this Jun 10, 2015
@hughker
Copy link
Author

hughker commented Jun 18, 2015

@Glavin001 seems to work pretty well. I see some things are breaking onto another line such as shown in your example with the }} above the </li> and with a few examples I tried. Better than nothing, that's for sure.

@Glavin001
Copy link
Owner

Great!

I am open to reviewing Pull Requests for adding this language and using Pretty Diff as the beautifier. See https://github.com/Glavin001/atom-beautify/blob/master/docs/add-languages-and-beautifiers.md#how-to-add-a-language for details, and let me know if anyone has any questions.

@prettydiff
Copy link
Collaborator

@hughker I am writing a new markup parser from scratch: http://prettydiff.com/ignore/testlocation/prettydiff.com.xhtml

That should provide superior support using half the code and execute 5x faster. Please let me know if you experience any bugs. The new parser is not released yet as I am still performing edge case testing.

@hughker
Copy link
Author

hughker commented Jun 18, 2015

@prettydiff the new markup parser works great! I used the same code sample provided by @Glavin001 and it provided the following code:

<ul id="products">
  {% for product in products %}
  <li>
    <h2>{{ product.name }}</h2>
    Only
    {{ product.price | price }}
    {{ product.description | prettyprint | paragraph }}
  </li>
  {% endfor %}
</ul>

Would the new parser still be able to integrate with this?

@prettydiff
Copy link
Collaborator

@hughker The new parser was released shortly after my prior comment. It should do everything you need.

@sheck
Copy link

sheck commented Feb 2, 2016

+1 for liquid support!

@macdonaldr93
Copy link

+1 for liquid support

@josephrlee
Copy link

+1 for liquid support!

@rptrk
Copy link

rptrk commented Mar 4, 2016

+1 for liquid support

@prettydiff
Copy link
Collaborator

You guys please try passing Liquid samples through http://prettydiff.com/?m=beautify and let me know if there are problems. If this works well then I will write the pull request.

@macdonaldr93
Copy link

@prettydiff definitely! Thank you.

@macdonaldr93
Copy link

@prettydiff link to provide feedback/track issues?

@prettydiff
Copy link
Collaborator

@macdonaldr93
Copy link

@prettydiff awesome! I'll get the team here using it and we can get some feedback for you.

@macdonaldr93
Copy link

@prettydiff what code type have you found works best with liquid? I've tried html and ERB Template and they both seem to work great.

@hughker
Copy link
Author

hughker commented Mar 4, 2016

@macdonaldr93 when I first opened this I tried ERB as well and it seemed to work the best IMO. I think I remember when I tested it out with more complex Liquid it seemed to have a tougher time. However, for the majority of the Liquid formatting it seems to do the trick.

@macdonaldr93
Copy link

@hughker sweet thanks! I'm going to keep testing and hopefully some other guys on the team will help out too so that we can get this into Atom Beautify.

@prettydiff
Copy link
Collaborator

Please feel free to open Pretty Diff issues for advanced defects. If I am made aware of problems I can provide an opinion or resolution. If I have no idea then it won't get any better.

@macdonaldr93
Copy link

@prettydiff I just want to confirm here that the intention of adding Liquid is to actually select Liquid as the language in Atom.

https://screenshot.click/04-06-vqzp3-j9nlj.png

We're probably going to develop our own syntax hightlighting so it would be nice if this was in it's own language rather than improved the existing ERB Template.

@macdonaldr93
Copy link

@prettydiff I've added two issues. I will continue to log any issues with the Liquid: prefix. Does that work for you?

@prettydiff
Copy link
Collaborator

Yes, a more specific language grammar would be ideal. It would be less ideal to identify this as ERB in the case that support for these languages diverge into the future.

Yes, the liquid issue prefix will be very helpful... thank you.

@macdonaldr93
Copy link

Great! Well I'll continue testing on a daily basis. Look forward to getting this liquid beautifier and language added to Atom.

@josephrlee
Copy link

@prettydiff Thanks for your work on this!

@prettydiff
Copy link
Collaborator

Support added to Pretty Diff v1.16.28.

@macdonaldr93
Copy link

You rock!

https://m.popkey.co/2b6361/VlXA6.gif

@prettydiff just out of curiosity, when would you expect this to be added to Atom Beautify?

@Glavin001
Copy link
Owner

@macdonaldr93 : You can uninstall and reinstall Atom Beautify to trigger a fresh and up-to-date installation of Pretty Diff dependency.

@prettydiff
Copy link
Collaborator

@macdonaldr93 You don't need any special grammar to trigger support. HTML grammar with Pretty Diff as the beautifier is good enough to get the full benefits. It would be nice to specify Liquid as a grammar (if it isn't already there) so as to call out support, though. The fastest way to get that though is to modify a couple files in Atom Beautify and submit a pull request.

@macdonaldr93
Copy link

@Glavin001 thanks! @prettydiff just to confirm though, the HTML grammar wouldn't be using the new Liquid Template code type though? That's where we'd have to add Liquid grammar to Atom Beautify?

@prettydiff
Copy link
Collaborator

@macdonaldr93 I only hide template features behind options and flags in the case of collisions. In this case the {% and {{ are unique enough from HTML, JavaScript, and CSS syntax that there isn't any collision so regular HTML containing this code parses the same as HTML specific to Liquid.

The only exception is when one of the delimiters occurs inside a JavaScript string and is not complete, for instance: "some string {% liquid code". This would cause a problem because it would attempt to parse and beautify the template code and insert the result back into that place in the string, but there is not a closing delimiter so the parser won't realize the string has finished before the Liquid code. I have some bounds checking to compensate for this as necessary to prevent recursive errors in beautifying Pretty Diff code.

@rickydazla
Copy link

@prettydiff that would be invalid Liquid syntax anyway since it is compiled before any javascript can run.

+1 for everything you are doing 👊

@prettydiff
Copy link
Collaborator

@rickydazla Pretty Diff has had some updates since my previous comment that may fix this and other issues. Would you mind passing some code through the tool (http://prettydiff.com/) and let me know if quickly spot defects.

@rickydazla
Copy link

@prettydiff yes: ✨ b.e.a.u.t.i.f.u.l!

@rickydazla
Copy link

rickydazla commented Jun 6, 2016

@prettydiff I did find an error when applying to a file of type .scss.liquid...

e.g. this:

margin: 0 {{ some_variable }}em;

.... became this:

margin: 0{{some_variable}}em;

The removal of spacing in between the curly braces is ok (although not really necessary, I think it looks nicer with), but the removal of space after the zero is an obvious issue.

@prettydiff
Copy link
Collaborator

@rickydazla I opened a defect for that minor item. I am currently away at a military school right now, so this weekend would be the earliest I could get to it. I have opened A LOT of issues since yesterday (first time being back online in 5 weeks), so I will get to it as quickly as I can.

@rickydazla
Copy link

Do what you gotta do brother man, I'll be sitting here writing fugly code!

@Glavin001 Glavin001 assigned prettydiff and unassigned Glavin001 Jul 3, 2016
@jason-engage
Copy link

Any clear instructions on how to add .LIQUID support? I don't see a Liquid option...

@Glavin001
Copy link
Owner

Instructions on adding a new language (such as Liquid) can be found at https://github.com/Glavin001/atom-beautify/blob/master/docs/add-languages-and-beautifiers.md#how-to-add-a-language
Let me know if you have any questions! Feel free to submit a Pull Request earlier than later so we can discuss changes, etc. Thanks in advance!

@prettydiff
Copy link
Collaborator

I am closing this issue as Liquid support is long since added to Pretty Diff. Please follow the guidance from Glavin for extending language support in Atom Beautify.

@Glavin001
Copy link
Owner

Glavin001 commented Apr 20, 2017

Pull Requests welcome! Let me know if you have any questions.

@bradonomics
Copy link

It's been a while since anyone has made comment on this. Is this still being worked on? I'm having formatting issues as described above.

The frontmatter is getting inlined as mentioned on issue #261. Liquid tags are also breaking as shown below:

Input Before Beautification

<section class="about">
      <div class="wrap">
        {% regionblock about_quote, type: text %}
                            <p>Some paragraph text.</p>
    {% endregionblock %}
      {% regionblock about_image, type: image %}
               <img src="/assets/images/about/about.jpg" class="img-fluid">
     {% endregionblock %}
            </div>
</section>

Expected Output

<section class="about">
  <div class="wrap">
    {% regionblock about_quote, type: text %}
      <p>Some paragraph text.</p>
    {% endregionblock %}
    {% regionblock about_image, type: image %}
      <img src="/assets/images/about/about.jpg" class="img-fluid">
    {% endregionblock %}
  </div>
</section>

Output Using JS Beautify

<section class="about">
  <div class="wrap">
    {% regionblock about_quote, type: text %}
    <p>Some paragraph text.</p>
    {% endregionblock %} {% regionblock about_image, type: image %}
    <img src="/assets/images/about/about.jpg" class="img-fluid"> {% endregionblock %}
  </div>
</section>

Output Using Pretty Diff

<section class="about">
  <div class="wrap">
    {% regionblock about_quote,
    type: text %}
    <p>Some paragraph text.</p>
    {% endregionblock %}
    {% regionblock about_image,
    type: image %}
    <img src="/assets/images/about/about.jpg" class="img-fluid">
    {% endregionblock %}
  </div>
</section>

I realize these are custom tags, but it would seem anything inside the {% %} tags would be unformatted.

Also, the above is only if I highlight the single section. When I try to beautify the entire file with frontmatter present, Pretty Diff errors:

Cannot convert undefined or null to object

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at markuppretty__beautify_apply_attArray (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/lib/markuppretty.js:3349:47)
    at markuppretty__beautify_apply (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/lib/markuppretty.js:3367:34)
    at markuppretty__beautify (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/lib/markuppretty.js:3479:20)
    at Object.markuppretty_ [as markuppretty] (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/lib/markuppretty.js:3480:10)
    at core__markuppretty (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/prettydiff.js:77:30)
    at core_ (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/prettydiff.js:307:37)
    at prettydiff_ (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/prettydiff.js:334:16)
    at commonjs_prettydiff (/home/brad/.atom/packages/atom-beautify/node_modules/prettydiff2/prettydiff.js:369:20)
    at /home/brad/.atom/packages/atom-beautify/src/beautifiers/prettydiff.coffee:128:16
    at Promise._execute (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/debuggability.js:303:9)
    at Promise._resolveFromExecutor (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/promise.js:483:18)
    at new Promise (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/promise.js:79:10)
    at PrettyDiff.module.exports.PrettyDiff.beautify (/home/brad/.atom/packages/atom-beautify/src/beautifiers/prettydiff.coffee:75:16)
    at /home/brad/.atom/packages/atom-beautify/src/beautifiers/index.coffee:355:28
    at tryCatcher (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromiseCtx (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/promise.js:606:10)
    at Async._drainQueue (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/async.js:138:12)
    at Async._drainQueues (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/async.js:143:10)
    at Async.drainQueues (/home/brad/.atom/packages/atom-beautify/node_modules/bluebird/js/release/async.js:17:14)

I've created a Gist of the full file if it's helpful.

Software Versions

OS

Linux (Ubuntu 17.10)

$ uname -r
4.13.0-21-generic

Atom

$ atom -v
Atom    : 1.22.1
Electron: 1.6.15
Chrome  : 56.0.2924.87
Node    : 7.4.0

Atom-Beautify

0.30.9

@Glavin001 Glavin001 modified the milestones: v0.31.0, v0.33.0 (Next) Mar 2, 2018
@petercampanelli
Copy link

Any update on this?

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

No branches or pull requests