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

removes windows newlines on windows machines #160

Closed
kellyselden opened this issue Oct 10, 2019 · 8 comments
Closed

removes windows newlines on windows machines #160

kellyselden opened this issue Oct 10, 2019 · 8 comments
Labels
bug Something isn't working

Comments

@kellyselden
Copy link
Contributor

Discovered here https://ci.appveyor.com/project/embercli/ember-cli-update/builds/28014446#L4814

  1) Acceptance | Ember-Cli-Update
       runs codemods:
      AssertionError: expected { Object (.editorconfig, .ember-cli, ...) } to deeply equal { Object (.editorconfig, .ember-cli, ...) }
      + expected - actual
           "styles": {
             "app.css": ""
           }
           "templates": {
      -      "application.hbs": "{{! The following component displays Ember's default welcome message. }}\n<WelcomePage />\n{{! Feel free to remove this! }}\n{{outlet}}\n<Ui::Button @text=\"Click me\" />"
      +      "application.hbs": "{{! The following component displays Ember's default welcome message. }}\r\n<WelcomePage />\r\n{{! Feel free to remove this! }}\r\n{{outlet}}\r\n<Ui::Button @text=\"Click me\" />"
             "components": {
               ".gitkeep": ""
             }
           }
      
      at fixtureCompare (node_modules\git-fixtures\src\index.js:252:26)
      at fixtureCompare (test\acceptance\ember-cli-update-test.js:101:5)
      at Context.<anonymous> (test\acceptance\ember-cli-update-test.js:171:5)
      at <anonymous>

@Turbo87
Copy link
Collaborator

Turbo87 commented Oct 15, 2019

this sounds like it might be an issue in ember-template-recast 🤔

in any case we should adjust CI to also run on Windows to catch these sorts of issues

@rwjblue
Copy link
Member

rwjblue commented Oct 15, 2019

Agree, though I didn't think it mucked with the line endings.

@rwjblue
Copy link
Member

rwjblue commented Oct 15, 2019

There are no locations in ember-template-recast that I can find where a newline is being added. I created ember-template-lint/ember-template-recast#141 to demonstrate that line endings are preserved.

@rwjblue
Copy link
Member

rwjblue commented Oct 15, 2019

I'm adding windows CI there too (in a separate PR)

@rwjblue
Copy link
Member

rwjblue commented Oct 16, 2019

Landed in ember-template-lint/ember-template-recast#143. There were a few specific changes that I needed to make, but none of them related to formatting of the actual output (they were related to how we selected files with globby, see ember-template-lint/ember-template-recast#147 for details on that one).

@rwjblue
Copy link
Member

rwjblue commented Oct 18, 2019

@kellyselden - We'll need more help to make progress here....

@rwjblue
Copy link
Member

rwjblue commented Nov 3, 2019

FWIW, I confirmed that:

  • this codemod does not add any line breaks (it preserves the block statement contents as is)
  • newly created nodes (that go through @glimmer/syntaxs printer) do not get any additional line endings added
  • ember-template-recast uses exactly the line endings that pre-existed in the file when a node is not mutated, and it does not directly add any line endings

I honestly do not know what is going on that is causing the issue above 😩

Has anyone been able to reproduce this outside of a CI environment? Any pointers on how we can dig in more?

@rwjblue
Copy link
Member

rwjblue commented Nov 4, 2019

Based on the diff pasted in ember-cli/ember-cli-update#606 (comment) I think this line ending issue is resolved (and has been for quite some time, but we didn't do a release until a week or two ago).

I suspect that it was reporting an issue with the prior major version of this codemod which still used prettier, and that prettier was the cause of the line ending differences.

Going to close now, definitely happy to reopen if I'm mistaken...

@rwjblue rwjblue closed this as completed Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants