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

Support whitespace control (~) #46

Closed
GavinJoyce opened this issue Apr 26, 2019 · 10 comments · Fixed by glimmerjs/glimmer-vm#951 or #181
Closed

Support whitespace control (~) #46

GavinJoyce opened this issue Apr 26, 2019 · 10 comments · Fixed by glimmerjs/glimmer-vm#951 or #181

Comments

@GavinJoyce
Copy link
Collaborator

The following input:

<div>
  {{~both~}}
</div>
<div>
  {{~left}}
</div>
<div>
  {{right~}}
</div>

results in the incorrect output:

<div>
  {{both}}
</div>
<div>
  {{left}}
</div>
<div>
  {{right}}
</div>

we should leave ~s in place

@tylerturdenpants
Copy link
Collaborator

You give me work, I’ll give you work!

@GavinJoyce
Copy link
Collaborator Author

It looks like the presence of ~s isn't included in the AST:

Screen Shot 2019-04-26 at 16 18 31

I wonder if we can use the location information to lookup the source and detect if there are preceding or trailing characters?

@rwjblue
Copy link
Member

rwjblue commented Apr 26, 2019

Ya, we can either use the source info to introspect for the ~ or we can make glimmer-vm include them in the generated AST. I'd vaguely prefer the latter.

Should be straight forward to add I think, see here:

https://github.com/glimmerjs/glimmer-vm/blob/157ffdb705699c8a03c7a4199af1779d2eea1b1f/packages/%40glimmer/syntax/lib/parser/handlebars-node-visitors.ts#L100

That location (and the BlockStatement one) receive the result of the handlebars parser, which has rightStripped / leftStripped boolean flags.

@GavinJoyce
Copy link
Collaborator Author

While we await this issue being resolved, I'm skipping files that contain a ~ character in https://github.com/rajasegar/ember-angle-brackets-codemod/pull/61

@rwjblue
Copy link
Member

rwjblue commented Jul 3, 2019

Just wanted to mention glimmerjs/glimmer-vm#951, I think its probably good to land but was hoping to get a conceptual review by folks first.

If no movement there in a few days, ping me again 😛 (I'm forgetful)...

@rwjblue
Copy link
Member

rwjblue commented Jul 4, 2019

The fix in glimmerjs/glimmer-vm#951 has landed (which auto-closed this issue), reopening to track updating here to account for the changes.

@Turbo87 Turbo87 changed the title ~s are being removed Support whitespace control (~) Oct 1, 2019
@tylerturdenpants
Copy link
Collaborator

tylerturdenpants commented Oct 24, 2019

There is a test in the 2.0 release that shows this is fixed:

test('whitespace-control', () => {
  let input = `
    <div>
      {{~both~}}
    </div>
    <div>
      {{~left}}
    </div>
    <div>
      {{right~}}
    </div>
  `;

  expect(runTest('whitespace-control.hbs', input)).toMatchInlineSnapshot(`
    "
        <div>
          {{~both~}}
        </div>
        <div>
          {{~left}}
        </div>
        <div>
          {{right~}}
        </div>
      "
  `);
});

@tylerturdenpants
Copy link
Collaborator

Sorry, spoke to soon, re-opening this issue

@tylerturdenpants
Copy link
Collaborator

I have opened this issue:
ember-template-lint/ember-template-recast#155

And this failing test:
ember-template-lint/ember-template-recast#156

@rwjblue
Copy link
Member

rwjblue commented Oct 28, 2019

Just released ember-template-recast@3.2.8 which fixes the issue mentioned above. The primary changes happened in glimmerjs/glimmer-vm#983 and handlebars-lang/handlebars.js#1584.

This lib will need to update to the latest ember-template-recast to get those fixes.

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