Skip to content

Only emit terminal codes to terminal (and don't require say)#221

Merged
creswick merged 2 commits into
awslabs:masterfrom
dannysauer:bug/no-require-terminal
Jun 20, 2023
Merged

Only emit terminal codes to terminal (and don't require say)#221
creswick merged 2 commits into
awslabs:masterfrom
dannysauer:bug/no-require-terminal

Conversation

@dannysauer
Copy link
Copy Markdown
Contributor

@dannysauer dannysauer commented Oct 13, 2022

Issue #, if available:
Fixes #220
Fixes #239

Description of changes:

  • Check for presence of tput and for stdout being a terminal before emitting terminal control codes.
  • Remove unnecessary dependence on say (which was removed: git/git@5b893f7)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check for presence of tput and for stdout being a terminal before emitting terminal control codes.
Remove unnecessary dependence on `say`
@dannysauer dannysauer force-pushed the bug/no-require-terminal branch from 9769e8c to 65891e2 Compare October 13, 2022 18:23
@cahrens
Copy link
Copy Markdown

cahrens commented Oct 20, 2022

It would be great to get some eyes on this PR, as we are sporadically hitting the problem of say not being present (we also see the terminal warning, but that doesn't cause a failure).

@dannysauer dannysauer changed the title Only emit terminal codes to terminal Only emit terminal codes to terminal (and don't require say) Oct 21, 2022
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 21, 2022
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 21, 2022
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 21, 2022
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 21, 2022
singholt added a commit to aws/amazon-ecs-agent that referenced this pull request Oct 22, 2022
@dannysauer
Copy link
Copy Markdown
Contributor Author

Is @mtdowling still affiliated with this project? Your name is all over the blame from seven years ago. :)

Trying to find some reviewers.

sparr
sparr previously approved these changes Jun 15, 2023
Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
Copy link
Copy Markdown
Contributor

@sparr sparr left a comment

Choose a reason for hiding this comment

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

I pushed a commit to use command instead of which, but otherwise this is a great change that resolves outstanding test failures.

@creswick creswick merged commit 0ab55f8 into awslabs:master Jun 20, 2023
@kdenhartog
Copy link
Copy Markdown

for those who are on MacOS and still encountering this problem, you'll need to use the make install method of installation. The brew cask has not been updated yet.

singholt added a commit to aws/amazon-ecs-agent that referenced this pull request Oct 11, 2023
the upstream issue with git secrets requiring "say" command has been fixed upstream: awslabs/git-secrets#221
singholt added a commit to aws/amazon-ecs-agent that referenced this pull request Oct 11, 2023
the upstream issue with git secrets requiring "say" command has been fixed upstream: awslabs/git-secrets#221
timj-hh pushed a commit to timj-hh/amazon-ecs-agent that referenced this pull request Oct 23, 2023
the upstream issue with git secrets requiring "say" command has been fixed upstream: awslabs/git-secrets#221
@irgaly
Copy link
Copy Markdown

irgaly commented May 2, 2024

Homebrew's git-secrets v1.3.0 still uses say command, so macOS users encounters VoiceOver's voice from your speaker when you run git secrets --install command.

brew link git-secrets --HEAD will run make on master branch and install git-secrets that does not contains say command problem.

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.

git-secrets --install test failures without code changes Installation fails without a terminal

7 participants