Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/commands/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
getToken,
log,
logJson,
prettyJsonRenderOptions,
warn,
type APIError,
} from '../../utils/command-helpers.js'
Expand Down Expand Up @@ -901,7 +902,7 @@ const printResults = ({
}),
)

log(prettyjson.render(msgData))
log(prettyjson.render(msgData, prettyJsonRenderOptions()))

if (!deployToProduction) {
log()
Expand Down Expand Up @@ -960,11 +961,14 @@ const prepAndRunDeploy = async ({

log('')
log(
prettyjson.render({
'Deploy path': deployFolder,
'Functions path': functionsFolder,
'Configuration path': configPath,
}),
prettyjson.render(
{
'Deploy path': deployFolder,
'Functions path': functionsFolder,
'Configuration path': configPath,
},
prettyJsonRenderOptions(),
),
)
log()

Expand Down
23 changes: 17 additions & 6 deletions src/commands/sites/sites-create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@ import inquirer from 'inquirer'
import { pick } from '../../utils/object-utilities.js'
import prettyjson from 'prettyjson'

import { chalk, logAndThrowError, log, logJson, warn, type APIError } from '../../utils/command-helpers.js'
import {
chalk,
logAndThrowError,
log,
logJson,
prettyJsonRenderOptions,
warn,
type APIError,
} from '../../utils/command-helpers.js'
import getRepoData from '../../utils/get-repo-data.js'
import { configureRepo } from '../../utils/init/config.js'
import { isInteractive } from '../../utils/scripted-commands.js'
Expand Down Expand Up @@ -158,11 +166,14 @@ export const sitesCreate = async (options: OptionValues, command: BaseCommand) =

const siteUrl = site.ssl_url || site.url
log(
prettyjson.render({
'Admin URL': site.admin_url,
URL: siteUrl,
'Project ID': site.id,
}),
prettyjson.render(
{
'Admin URL': site.admin_url,
URL: siteUrl,
'Project ID': site.id,
},
prettyJsonRenderOptions(),
),
)

track('sites_created', {
Expand Down
4 changes: 2 additions & 2 deletions src/commands/status/status-hooks.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { OptionValues } from 'commander'
import prettyjson from 'prettyjson'

import { log } from '../../utils/command-helpers.js'
import { log, prettyJsonRenderOptions } from '../../utils/command-helpers.js'
import type BaseCommand from '../base-command.js'

interface StatusHook {
Expand Down Expand Up @@ -40,5 +40,5 @@ export const statusHooks = async (_options: OptionValues, command: BaseCommand):
log(`─────────────────┐
Project Hook Status │
─────────────────┘`)
log(prettyjson.render(data))
log(prettyjson.render(data, prettyJsonRenderOptions()))
}
20 changes: 12 additions & 8 deletions src/commands/status/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getToken,
log,
logJson,
prettyJsonRenderOptions,
warn,
type APIError,
} from '../../utils/command-helpers.js'
Expand Down Expand Up @@ -67,7 +68,7 @@ export const status = async (options: OptionValues, command: BaseCommand) => {
// another lib.
(clean as unknown as <T extends Record<string | number | symbol, unknown>>(obj: T) => Partial<T>)(accountData)

log(prettyjson.render(cleanAccountData))
log(prettyjson.render(cleanAccountData, prettyJsonRenderOptions()))

if (!siteId) {
warn('Did you run `netlify link` yet?')
Expand All @@ -92,13 +93,16 @@ export const status = async (options: OptionValues, command: BaseCommand) => {
Netlify Project Info │
────────────────────┘`)
log(
prettyjson.render({
'Current project': siteInfo.name,
'Netlify TOML': site.configPath,
'Admin URL': chalk.magentaBright(siteInfo.admin_url),
'Project URL': chalk.cyanBright(siteInfo.ssl_url || siteInfo.url),
'Project Id': chalk.yellowBright(siteInfo.id),
}),
prettyjson.render(
{
'Current project': siteInfo.name,
'Netlify TOML': site.configPath,
'Admin URL': chalk.magentaBright(siteInfo.admin_url),
'Project URL': chalk.cyanBright(siteInfo.ssl_url || siteInfo.url),
'Project Id': chalk.yellowBright(siteInfo.id),
},
prettyJsonRenderOptions(),
),
)
log()
}
13 changes: 8 additions & 5 deletions src/commands/watch/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import pWaitFor from 'p-wait-for'
import prettyjson from 'prettyjson'

import { type Spinner, startSpinner, stopSpinner } from '../../lib/spinner.js'
import { chalk, logAndThrowError, log } from '../../utils/command-helpers.js'
import { chalk, logAndThrowError, log, prettyJsonRenderOptions } from '../../utils/command-helpers.js'
import type BaseCommand from '../base-command.js'
import { init } from '../init/init.js'

Expand Down Expand Up @@ -98,10 +98,13 @@ export const watch = async (_options: unknown, command: BaseCommand) => {
log()
log(message)
log(
prettyjson.render({
URL: siteData.ssl_url || siteData.url,
Admin: siteData.admin_url,
}),
prettyjson.render(
{
URL: siteData.ssl_url || siteData.url,
Admin: siteData.admin_url,
},
prettyJsonRenderOptions(),
),
)
console.timeEnd('Deploy time')
} catch (error_) {
Expand Down
10 changes: 9 additions & 1 deletion src/utils/command-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@ const safeChalk = function (noColors: boolean) {
return new Chalk()
}

export const chalk = safeChalk(argv.includes('--json'))
/** Honors the NO_COLOR convention (https://no-color.org): any non-empty value disables colors */
export const shouldDisableColors = (env: NodeJS.ProcessEnv = process.env): boolean => Boolean(env.NO_COLOR)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

chalk already implements all the various colour detection stuff (much more than just NO_COLOR), the problem is that we're overriding that entirely by passing argv.includes('--json'). chalk exports supportsColor so we should be able to just import that and change to something like argv.includes('--json') || supportsColor (that probably isn't right, but you get the idea).

https://main.npmx.dev/package/chalk#user-content-supportscolor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good instinct, but I checked and chalk doesn't actually handle NO_COLOR. It's a deliberate omission on their side.

The supports-color logic chalk 5.6.2 ships (vendored at node_modules/chalk/source/vendor/supports-color/index.js) checks --no-color/--color flags, FORCE_COLOR, TTY status, CI env vars, and TERM, but there is zero mention of NO_COLOR anywhere in the file. The chalk maintainers have declined to support it for years and point people to FORCE_COLOR=0 instead (e.g. chalk/supports-color#105). So argv.includes('--json') || !supportsColor would silently drop the NO_COLOR behavior this PR exists to add.

Also a small clarification on the "overriding entirely" part: we only override in the force-off case. When the arg is false, safeChalk returns new Chalk(), which still runs chalk's full auto-detection, so TTY/CI/FORCE_COLOR handling is already intact for chalk output today.

That said, your suggestion does point at a real gap for prettyjson: it does its own coloring and knows nothing about chalk's detection, so piped netlify status output gets ANSI codes today even without NO_COLOR. Pulling in supportsColor there in addition to (not instead of) the NO_COLOR check would fix that:

noColor: argv.includes('--json') || shouldDisableColors() || !supportsColor.stdout

Happy to add that here if you're on board, or split it into a follow-up.


export const chalk = safeChalk(argv.includes('--json') || shouldDisableColors())

/** Options for `prettyjson.render()` so human-formatted output honors NO_COLOR and --json like `chalk` does */
export const prettyJsonRenderOptions = (): { noColor: boolean } => ({
noColor: argv.includes('--json') || shouldDisableColors(),
})

export type ChalkInstance = ChalkInstancePrimitiveType

Expand Down
28 changes: 28 additions & 0 deletions tests/unit/utils/no-color.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { afterEach, describe, expect, test, vi } from 'vitest'

import { shouldDisableColors } from '../../../src/utils/command-helpers.js'

describe('NO_COLOR support', () => {
afterEach(() => {
vi.unstubAllEnvs()
vi.resetModules()
})

test('shouldDisableColors is true for any non-empty NO_COLOR value', () => {
expect(shouldDisableColors({ NO_COLOR: '1' })).toBe(true)
expect(shouldDisableColors({ NO_COLOR: 'true' })).toBe(true)
expect(shouldDisableColors({ NO_COLOR: '0' })).toBe(true)
})

test('shouldDisableColors is false when NO_COLOR is unset or empty', () => {
expect(shouldDisableColors({})).toBe(false)
expect(shouldDisableColors({ NO_COLOR: '' })).toBe(false)
})

test('chalk is initialized colorless when NO_COLOR is set', async () => {
vi.stubEnv('NO_COLOR', '1')
const { chalk } = await import('../../../src/utils/command-helpers.js')
expect(chalk.level).toBe(0)
expect(chalk.red('plain')).toBe('plain')
})
})
Loading