-
Notifications
You must be signed in to change notification settings - Fork 25
fix: disable ANSI colors in history list when stdout is not a TTY #777
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
Changes from 5 commits
5883e4e
c6d57e1
ccdf388
7241cdf
cc5f15a
4175946
97386b2
a971dbb
a50afad
e752727
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,9 @@ import chalk from 'chalk' | |
| import { randomUUID } from 'crypto' | ||
| import { describeCommand, invokeTestCli } from '../utility' | ||
| import { getStampOption } from '../utility/stamp' | ||
| import colors from '@colors/colors/safe' | ||
|
|
||
| const ANSI_PATTERN = /\u001B\[\d+m/ //adding this for testing the ansi disable part | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. total nit picking, but since this one is only related to the test cases L131-L166, I would move this const to that block
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
|
||
| async function uploadTestFile() { | ||
| const uploadFilePath = `${__dirname}/../testpage/images/swarm.png` | ||
|
|
@@ -124,6 +127,43 @@ describeCommand( | |
| await invokeTestCli(['history', 'disable', '--yes']) | ||
| }) | ||
| }) | ||
|
|
||
| describe('ansi: list colors', () => { | ||
| const originalIsTTY = process.stdout.isTTY | ||
| const colorsWereEnabled = colors.enabled | ||
|
|
||
| beforeAll(() => { | ||
| colors.enable() | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| if (!colorsWereEnabled) { | ||
| colors.disable() | ||
| } | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| process.stdout.isTTY = originalIsTTY | ||
| invokeTestCli(['history', 'disable', '--yes']) | ||
| }) | ||
|
|
||
| it('should not use colors when stdout is not a TTY', async () => { | ||
| await invokeTestCli(['history', 'enable']) | ||
| process.stdout.isTTY = false | ||
| await invokeTestCli(['history', 'list']) | ||
|
|
||
| expect(consoleMessages[1]).not.toMatch(ANSI_PATTERN) | ||
| expect(consoleMessages[1]).toContain('Timestamp') | ||
| }) | ||
|
|
||
| it('should use colors when stdout is a TTY', async () => { | ||
| await invokeTestCli(['history', 'enable']) | ||
| process.stdout.isTTY = true | ||
| await invokeTestCli(['history', 'list']) | ||
|
|
||
| expect(consoleMessages[1]).toMatch(ANSI_PATTERN) | ||
| }) | ||
| }) | ||
| }, | ||
| { configFileName: 'history' }, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please cover the
isTTY = falsecase with spec(s)?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests to history.spec.ts file