Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed command output echoing to preserve spaces #49

Merged
merged 1 commit into from
Sep 15, 2021
Merged

Fixed command output echoing to preserve spaces #49

merged 1 commit into from
Sep 15, 2021

Conversation

cederberg
Copy link
Contributor

Using IFS= when reading lines with read ensures that Bash doesn't split words on any characters, but instead returns each line as read. This preserves command output properly, with indentation preserved.

Without this change, command output of pretty-printed JSON looks quite ugly for instance.

@chrismytton
Copy link
Owner

@cederberg Thanks for this! ✨

Are you able to add a test for this change at all? Hopefully the examples in test/shoreman_test.sh will give you some guidance.

If you don't think you'll be able to write a test that's fine, just let me know and I can write a test when I have time.

Thanks again!

Using `IFS= ` when reading lines with `read` ensures that Bash doesn't split words on any characters, but instead returns each line as read. This preserves command output properly, with indentation preserved.
@cederberg
Copy link
Contributor Author

Test is now added and verified (i.e breaks on master but not on branch).

Thank you for this great utility!

Copy link
Owner

@chrismytton chrismytton left a comment

Choose a reason for hiding this comment

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

@cederberg Thanks for adding the test, think this is ready to go! 🚀

@chrismytton chrismytton merged commit 91fca54 into chrismytton:master Sep 15, 2021
@chrismytton
Copy link
Owner

chrismytton commented Sep 15, 2021

Have tagged v1.1.0 which contains this changes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants