Skip to content

scripts/warn-outside-container: fix escapes #4211

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kolyshkin
Copy link
Contributor

By default, echo does not interpret escape sequences, resulting in the following output (literal):

\033[1mWARNING\033[0m: you are not in a container.

I don't think this was intended.

Add -e option to echo so it can work as intended.

Also, use bash instead of sh because in POSIX shell echo does not take any options.

Fixes: 94e08f2

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

By default, echo does not interpret escape sequences,
resulting in the following output (literal):

	\033[1mWARNING\033[0m: you are not in a container.

I don't think this was intended.

Add -e option to echo so it can work as intended.

Also, use bash instead of sh because in POSIX shell echo does not take
any options.

Fixes: 94e08f2
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #4211 (2b8dbb6) into master (67c4570) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4211   +/-   ##
=======================================
  Coverage   58.86%   58.86%           
=======================================
  Files         572      572           
  Lines       49544    49544           
=======================================
  Hits        29162    29162           
  Misses      18616    18616           
  Partials     1766     1766           

@@ -1,4 +1,4 @@
#!/usr/bin/env sh
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Was there anything in the script requiring bash now, or would sh still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quoting myself

Also, use bash instead of sh because in POSIX shell echo does not take any options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alternative is to change all echo calls to printf (and add \n everywhere).

@thaJeztah
Copy link
Member

Still curious why it "worked on my machine" 🤔 Trying to run a make binary, I do see the output on both fish, bash, and zsh, or would they perhaps not enable POSIX mode on /bin/sh?

Screenshot 2023-04-25 at 12 15 26

Screenshot 2023-04-25 at 12 16 41

Screenshot 2023-04-25 at 12 18 26

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

Successfully merging this pull request may close these issues.

3 participants