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

Treat dumb terminals as noninteractive #5237

Merged
merged 2 commits into from
Jan 5, 2018
Merged

Conversation

nickserv
Copy link
Contributor

@nickserv nickserv commented Jan 5, 2018

Summary

Fixes #5236 and related issues outside of Emacs. Essentially, the issue is that isInteractive only checks process.stdin.isTTY and CI environment variables, but it should treat a shell with TERM set to dumb as noninteractive even if is has a TTY (not all TTYs support interactive escape codes). Note that while Emacs' Compilation mode supports interactive comint processes with a prefix argument, it is still not supported with this Jest patch (for example jest --watch will ignore input).

Test plan

  • yarn test packages/jest-util/src/__tests__/is_interactive.test.js passes
  • Running yarn link and jest in another project in Emacs successfully hides mode escape codes (though is an extra �[999D right after the last PASS line that I can't figure out how to remove, I'd appreciate feedback)

@codecov-io
Copy link

codecov-io commented Jan 5, 2018

Codecov Report

Merging #5237 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5237   +/-   ##
=======================================
  Coverage   61.18%   61.18%           
=======================================
  Files         202      202           
  Lines        6765     6765           
  Branches        3        4    +1     
=======================================
  Hits         4139     4139           
  Misses       2625     2625           
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-util/src/is_interative.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8ffd68...86eec66. Read the comment docs.

@@ -1,3 +1,3 @@
import isCI from 'is-ci';

export default process.stdout.isTTY && !isCI;
export default process.stdout.isTTY && process.env.TERM !== 'dumb' && !isCI;
Copy link
Member

Choose a reason for hiding this comment

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

Feels like there should exist some module which has this logic already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same feeling, but I couldn't find one that specifically checks for a dumb terminal without doing other stuff. For example supports-color takes into account if a terminal is dumb but it's coupled with color related features that we don't want to check in this case.

@cpojer cpojer merged commit 9cbc26b into jestjs:master Jan 5, 2018
@cpojer
Copy link
Member

cpojer commented Jan 5, 2018

Thanks for submitting a well detailed issues and PR to Jest to fix an issue you were running into!

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jest should treat some Emacs buffers as noninteractive
5 participants