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

Trim whitespace from file contents #186

Merged
merged 5 commits into from
Jan 7, 2022

Conversation

glesica
Copy link
Contributor

@glesica glesica commented Dec 17, 2021

I noticed that the file matcher I added last year doesn't trim whitespace, whereas the executors do trim whitespace from the content they capture from stdout and stderr. This causes annoyance if you edit the "gold" file with an editor like Vim that automatically appends a newline. I also changed it to convert \r\n to \n to match the executors.

Checklist

  • Added unit / integration tests for windows, macOS and Linux?
  • Added a changelog entry in CHANGELOG.md?
  • Updated the documentation (README.md, docs)?
  • Does your change work on Linux, Windows and macOS?

@glesica
Copy link
Contributor Author

glesica commented Jan 5, 2022

@dylanhitt @SimonBaeumer ping :-)

Copy link
Member

@dylanhitt dylanhitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I seemed to miss this juggling some things.

Nice - super informative with the \r\n

@dylanhitt
Copy link
Member

@glesica would you mind force pushing your last commit. I just setup circleci to build forked PRs.

@glesica
Copy link
Contributor Author

glesica commented Jan 7, 2022

Not sure what's failing, exactly...

@dylanhitt
Copy link
Member

So code coverage is failing to do forks not being allowed to run env variables which is fine.

The integration is failing because of the linecount not matching. You added a integration test so commander_unix.yaml the test it should execute tests needs to be iterated by 1 😄

@glesica
Copy link
Contributor Author

glesica commented Jan 7, 2022

Oh that's right! I'd forgotten about those! Done!

@dylanhitt dylanhitt merged commit b5954ca into commander-cli:master Jan 7, 2022
@glesica glesica deleted the trim-file branch January 20, 2022 19:00
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