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

refactor: Error handling #284

Merged
merged 36 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
bc95bfd
feat(cli): add --quiet / -q option
aarondill Jan 21, 2023
4becd52
refactor(cli): create output functions
aarondill Jan 22, 2023
0037f07
fix(cli): change stderr to not respect --quiet
aarondill Jan 23, 2023
a1042b9
fix(cli): change stdout to stderr in isTerminal
aarondill Jan 23, 2023
dd596cd
fix(tests): add STD(OUT/ERR)_IS_TTY env variables
aarondill Jan 23, 2023
6417a87
fix(cli): improve output of CLI when not a tty
aarondill Jan 23, 2023
cb4147e
test: added tests for --verson and --help
aarondill Jan 23, 2023
bc8d156
test: add tests for --quiet
aarondill Jan 23, 2023
b03c0f8
test: include isTerminal in snapshot
aarondill Jan 23, 2023
864e834
test: add tests for TTY detection and integration
aarondill Jan 23, 2023
296e580
test: typo, stderr --> stdout
aarondill Jan 23, 2023
192e4ca
fix(cli): exit code while sort is number of failed
aarondill Jan 23, 2023
7f1d607
fix: wrap file operation in try catch
aarondill Jan 23, 2023
7635316
docs: document changes to tty-based output
aarondill Jan 23, 2023
c517e46
fix(test): compatability w/ node v14
aarondill Jan 23, 2023
f425764
fix: compatability with node 12
aarondill Jan 23, 2023
aae2cdb
test: add tests for improper usage
aarondill Jan 23, 2023
5bc8c51
test: support for node 12&14 in error test
aarondill Jan 23, 2023
ea61369
refactor: remove extra changes to improve diff
aarondill Jan 30, 2023
411f897
revert: remove all tty detection
aarondill Jan 30, 2023
4be3963
refactor: update to meet upstream
aarondill Jan 30, 2023
bc4717c
Merge remote-tracking branch 'upstream/master' into error_handling
aarondill Jan 30, 2023
6658908
fix: fixes error reporting with quiet
aarondill Jan 30, 2023
9e34665
Merge branch 'master' into error_handling
keithamus Jan 30, 2023
a0c3676
fix: bad merge
keithamus Jan 30, 2023
042469c
style: prettier
keithamus Jan 30, 2023
4486d40
fix: fixes permissions on cli.js
aarondill Jan 30, 2023
7910160
typo
fisker Jan 31, 2023
7ac6327
refactor: improve exit code handling, and set 2 on error
aarondill Jan 31, 2023
1448bbc
feat: added summary at end of tool run
aarondill Feb 1, 2023
741010e
fix: better show that output on error, is an error
aarondill Feb 1, 2023
c5dd2cc
refactor: cleaner logic
fisker Feb 1, 2023
acfbca0
refactor: pass `files` to `constructor`
fisker Feb 1, 2023
ffcb3de
refactor: save `matchedFilesCount` to status
fisker Feb 1, 2023
b22d474
fix: remove `0 files`, use `1 file` instead of `1 files`
fisker Feb 1, 2023
972be64
refactor: extract `Reporter`
fisker Feb 1, 2023
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
166 changes: 133 additions & 33 deletions cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,52 +26,152 @@ If file/glob is omitted, './package.json' file will be processed.
)
}

function sortPackageJsonFiles(patterns, { isCheck, shouldBeQuiet }) {
const files = globbySync(patterns)
const printToStdout = shouldBeQuiet ? () => {} : console.log
const getFilesCountText = (count) => (count === 1 ? '1 file' : `${count} files`)
class Reporter {
#hasPrinted = false
#options
#status
#logger

constructor(files, options) {
this.#options = options
this.#status = {
matchedFilesCount: files.length,
failedFilesCount: 0,
wellSortedFilesCount: 0,
changedFilesCount: 0,
}

if (files.length === 0) {
console.error('No matching files.')
process.exitCode = 2
return
this.#logger = options.shouldBeQuiet
? { log() {}, error() {} }
: {
log: (...args) => {
this.#hasPrinted = true
console.log(...args)
},
error: (...args) => {
this.#hasPrinted = true
console.error(...args)
},
}
}

let notSortedFiles = 0
for (const file of files) {
const packageJson = fs.readFileSync(file, 'utf8')
const sorted = sortPackageJson(packageJson)
// The file is well-sorted
reportNotChanged(/* file */) {
this.#status.wellSortedFilesCount++
}

reportChanged(file) {
this.#status.changedFilesCount++
this.#logger.log(this.#options.isCheck ? `${file}` : `${file} is sorted!`)
}

reportFailed(file, error) {
this.#status.failedFilesCount++

console.error('Error on: ' + file)
this.#logger.error(error.message)
}

printSummary() {
const {
matchedFilesCount,
failedFilesCount,
changedFilesCount,
wellSortedFilesCount,
} = this.#status

if (matchedFilesCount === 0) {
console.error('No matching files.')
process.exitCode = 2
return
}

const { isCheck, isQuiet } = this.#options

if (isCheck && changedFilesCount) {
process.exitCode = 1
}

if (failedFilesCount) {
process.exitCode = 2
}

if (isQuiet) {
return
}

const { log } = this.#logger

// Print an empty line.
if (this.#hasPrinted) {
log()
}

if (sorted !== packageJson) {
// Matched files
log('Found %s.', getFilesCountText(matchedFilesCount))

// Failed files
if (failedFilesCount) {
log(
'%s could not be %s.',
getFilesCountText(failedFilesCount),
isCheck ? 'checked' : 'sorted',
)
}

// Changed files
if (changedFilesCount) {
if (isCheck) {
notSortedFiles++
printToStdout(file)
process.exitCode = 1
log(
'%s %s not sorted.',
getFilesCountText(changedFilesCount),
changedFilesCount === 1 ? 'was' : 'were',
)
} else {
fs.writeFileSync(file, sorted)

printToStdout(`${file} is sorted!`)
log('%s successfully sorted.', getFilesCountText(changedFilesCount))
}
}

// Well-sorted files
if (wellSortedFilesCount) {
log(
'%s %s already sorted.',
getFilesCountText(wellSortedFilesCount),
wellSortedFilesCount === 1 ? 'was' : 'were',
)
}
}
}

if (isCheck) {
// Print a empty line
printToStdout()
function sortPackageJsonFile(file, reporter, isCheck) {
const original = fs.readFileSync(file, 'utf8')
const sorted = sortPackageJson(original)
if (sorted === original) {
return reporter.reportNotChanged(file)
}

if (notSortedFiles) {
printToStdout(
notSortedFiles === 1
? `${notSortedFiles} of ${files.length} matched file is not sorted.`
: `${notSortedFiles} of ${files.length} matched files are not sorted.`,
)
} else {
printToStdout(
files.length === 1
? `${files.length} matched file is sorted.`
: `${files.length} matched files are sorted.`,
)
if (!isCheck) {
fs.writeFileSync(file, sorted)
}

reporter.reportChanged(file)
}

function sortPackageJsonFiles(patterns, options) {
const files = globbySync(patterns)
const reporter = new Reporter(files, options)
const { isCheck } = options

for (const file of files) {
try {
sortPackageJsonFile(file, reporter, isCheck)
} catch (error) {
reporter.reportFailed(file, error)
}
}

reporter.printSummary()
}

function run() {
Expand Down
52 changes: 51 additions & 1 deletion tests/cli.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fs from 'node:fs'
import test from 'ava'
import fs from 'node:fs'
import { cliScript, macro } from './_helpers.js'

const badJson = {
Expand Down Expand Up @@ -438,3 +438,53 @@ test('run `cli --check --quiet` on duplicate patterns', macro.testCLI, {
],
message: 'Should not count `bad-1/package.json` more than once. Exit code 1',
})

const badFormat = ''

test('run `cli --check` on 1 non-json file', macro.testCLI, {
fixtures: [
{
file: 'notJson/package.json',
content: badFormat,
expect: badFormat,
},
],
args: ['*/package.json', '--check'],
message: 'Should fail to check, but not end execution.',
})

test('run `cli --check --quiet` on 1 non-json file', macro.testCLI, {
fixtures: [
{
file: 'notJson/package.json',
content: badFormat,
expect: badFormat,
},
],
args: ['*/package.json', '--check', '--quiet'],
message: 'Should output error message, but not count.',
})

test('run `cli` on 1 non-json file', macro.testCLI, {
fixtures: [
{
file: 'notJson/package.json',
content: badFormat,
expect: badFormat,
},
],
args: ['*/package.json'],
message: 'Should fail to check, but not end execution.',
})

test('run `cli --quiet` on 1 non-json file', macro.testCLI, {
fixtures: [
{
file: 'notJson/package.json',
content: badFormat,
expect: badFormat,
},
],
args: ['*/package.json', '--quiet'],
message: 'Should output error message',
})
Loading