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

feat: --no-stash flag implies --no-hide-partially-staged #1371

Merged
merged 1 commit into from
Dec 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/empty-gifts-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'lint-staged': minor
---

Using the `--no-stash` flag no longer discards all unstaged changes to partially staged files, which resulted in inadvertent data loss. This fix is available with a new flag `--no-hide-partially-staged` that is automatically enabled when `--no-stash` is used.
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ Options:
"--no-stash".
--diff-filter [string] override the default "--diff-filter=ACMR" flag of "git diff" to get list of files
--max-arg-length [number] maximum length of the command-line argument string (default: 0)
--no-stash disable the backup stash, and do not revert in case of errors
--no-stash disable the backup stash, and do not revert in case of errors. Implies
"--no-hide-partially-staged".
--no-hide-partially-staged disable hiding unstaged changes from partially staged files
-q, --quiet disable lint-staged’s own console output (default: false)
-r, --relative pass relative filepaths to tasks (default: false)
-x, --shell [path] skip parsing of tasks for better shell support (default: false)
Expand All @@ -140,7 +142,8 @@ Options:
- **`--diff`**: By default linters are filtered against all files staged in git, generated from `git diff --staged`. This option allows you to override the `--staged` flag with arbitrary revisions. For example to get a list of changed files between two branches, use `--diff="branch1...branch2"`. You can also read more from about [git diff](https://git-scm.com/docs/git-diff) and [gitrevisions](https://git-scm.com/docs/gitrevisions). This option also implies `--no-stash`.
- **`--diff-filter`**: By default only files that are _added_, _copied_, _modified_, or _renamed_ are included. Use this flag to override the default `ACMR` value with something else: _added_ (`A`), _copied_ (`C`), _deleted_ (`D`), _modified_ (`M`), _renamed_ (`R`), _type changed_ (`T`), _unmerged_ (`U`), _unknown_ (`X`), or _pairing broken_ (`B`). See also the `git diff` docs for [--diff-filter](https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---diff-filterACDMRTUXB82308203).
- **`--max-arg-length`**: long commands (a lot of files) are automatically split into multiple chunks when it detects the current shell cannot handle them. Use this flag to override the maximum length of the generated command string.
- **`--no-stash`**: By default a backup stash will be created before running the tasks, and all task modifications will be reverted in case of an error. This option will disable creating the stash, and instead leave all modifications in the index when aborting the commit. Can be re-enabled with `--stash`.
- **`--no-stash`**: By default a backup stash will be created before running the tasks, and all task modifications will be reverted in case of an error. This option will disable creating the stash, and instead leave all modifications in the index when aborting the commit. Can be re-enabled with `--stash`. This option also implies `--no-hide-partially-staged`.
- **`--no-hide-partially-staged`**: By default, unstaged changes from partially staged files will be hidden. This option will disable this behavior and include all unstaged changes in partially staged files. Can be re-enabled with `--hide-partially-staged`
- **`--quiet`**: Supress all CLI output, except from tasks.
- **`--relative`**: Pass filepaths relative to `process.cwd()` (where `lint-staged` runs) to tasks. Default is `false`.
- **`--shell`**: By default linter commands will be parsed for speed and security. This has the side-effect that regular shell scripts might not work as expected. You can skip parsing of commands with this option. To use a specific shell, use a path like `--shell "/bin/bash"`.
Expand Down
22 changes: 21 additions & 1 deletion bin/lint-staged.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,25 @@ cli
.addOption(
new Option(
'--no-stash',
'disable the backup stash, and do not revert in case of errors'
'disable the backup stash, and do not revert in case of errors. Implies "--no-hide-partially-staged".'
).default(false)
)

/**
* We don't want to show the `--hide-partially-staged` flag because it's on by default, and only show the
* negatable flag `--no-hide-partially-staged` in stead. There seems to be a bug in Commander.js where
* configuring only the latter won't actually set the default value.
*/
cli
.addOption(
new Option('--hide-partially-staged', 'hide unstaged changes from partially staged files')
.default(null)
.hideHelp()
)
.addOption(
new Option(
'--no-hide-partially-staged',
'disable hiding unstaged changes from partially staged files'
).default(false)
)

Expand Down Expand Up @@ -104,6 +122,8 @@ const options = {
relative: !!cliOptions.relative,
shell: cliOptions.shell /* Either a boolean or a string pointing to the shell */,
stash: !!cliOptions.stash, // commander inverts `no-<x>` flags to `!x`
hidePartiallyStaged:
cliOptions.hidePartiallyStaged == null ? !!cliOptions.stash : !!cliOptions.hidePartiallyStaged, // commander inverts `no-<x>` flags to `!x`
verbose: !!cliOptions.verbose,
}

Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const lintStaged = async (
shell = false,
// Stashing should be disabled by default when the `diff` option is used
stash = diff === undefined,
hidePartiallyStaged = stash,
verbose = false,
} = {},
logger = console
Expand All @@ -101,6 +102,7 @@ const lintStaged = async (
relative,
shell,
stash,
hidePartiallyStaged,
verbose,
}

Expand Down
13 changes: 13 additions & 0 deletions lib/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ export const skippingBackup = (hasInitialCommit, diff) => {
return chalk.yellow(`${warning} Skipping backup because ${reason}.\n`)
}

export const skippingHidePartiallyStaged = (stash, diff) => {
const reason =
diff !== undefined
? '`--diff` was used'
: !stash
? '`--no-stash` was used'
: '`--no-hide-partially-staged` was used'

return chalk.yellow(
`${warning} Skipping hiding unstaged changes from partially staged files because ${reason}.\n`
)
}

export const DEPRECATED_GIT_ADD = chalk.yellow(
`${warning} Some of your tasks use \`git add\` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index.
`
Expand Down
13 changes: 10 additions & 3 deletions lib/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
NO_TASKS,
SKIPPED_GIT_ERROR,
skippingBackup,
skippingHidePartiallyStaged,
} from './messages.js'
import { normalizePath } from './normalizePath.js'
import { resolveGitRepo } from './resolveGitRepo.js'
Expand All @@ -30,7 +31,7 @@ import {
cleanupEnabled,
cleanupSkipped,
getInitialState,
hasPartiallyStagedFiles,
shouldHidePartiallyStagedFiles,
restoreOriginalStateEnabled,
restoreOriginalStateSkipped,
restoreUnstagedChangesSkipped,
Expand Down Expand Up @@ -79,6 +80,7 @@ export const runAll = async (
shell = false,
// Stashing should be disabled by default when the `diff` option is used
stash = diff === undefined,
hidePartiallyStaged = stash,
verbose = false,
},
logger = console
Expand Down Expand Up @@ -112,6 +114,11 @@ export const runAll = async (
logger.warn(skippingBackup(hasInitialCommit, diff))
}

ctx.shouldHidePartiallyStaged = hidePartiallyStaged
if (!ctx.shouldHidePartiallyStaged && !quiet) {
logger.warn(skippingHidePartiallyStaged(hasInitialCommit && stash, diff))
}

const files = await getStagedFiles({ cwd: gitDir, diff, diffFilter })
if (!files) {
if (!quiet) ctx.output.push(FAILED_GET_STAGED_FILES)
Expand Down Expand Up @@ -280,7 +287,7 @@ export const runAll = async (
{
title: 'Hiding unstaged changes to partially staged files...',
task: (ctx) => git.hideUnstagedChanges(ctx),
enabled: hasPartiallyStagedFiles,
enabled: shouldHidePartiallyStagedFiles,
},
{
title: `Running tasks for ${diff ? 'changed' : 'staged'} files...`,
Expand All @@ -295,7 +302,7 @@ export const runAll = async (
{
title: 'Restoring unstaged changes to partially staged files...',
task: (ctx) => git.restoreUnstagedChanges(ctx),
enabled: hasPartiallyStagedFiles,
enabled: shouldHidePartiallyStagedFiles,
skip: restoreUnstagedChangesSkipped,
},
{
Expand Down
4 changes: 3 additions & 1 deletion lib/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ import {
export const getInitialState = ({ quiet = false } = {}) => ({
hasPartiallyStagedFiles: null,
shouldBackup: null,
shouldHidePartiallyStaged: true,
errors: new Set([]),
events: new EventEmitter(),
output: [],
quiet,
})

export const hasPartiallyStagedFiles = (ctx) => ctx.hasPartiallyStagedFiles
export const shouldHidePartiallyStagedFiles = (ctx) =>
ctx.hasPartiallyStagedFiles && ctx.shouldHidePartiallyStaged

export const applyModificationsSkipped = (ctx) => {
// Always apply back unstaged modifications when skipping backup
Expand Down
11 changes: 11 additions & 0 deletions test/integration/__fixtures__/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ export const prettyJS = `module.exports = {
};
`

export const prettyJSWithChanges = `module.exports = {
foo: "bar",
bar: "baz",
};
`

export const uglyJS = `module.exports = {
'foo': 'bar'
}
`
export const uglyJSWithChanges = `module.exports = {
'foo': 'bar',
'bar': 'baz'
}
`

export const invalidJS = `const obj = {
'foo': 'bar'
Expand Down
36 changes: 36 additions & 0 deletions test/integration/no-hide-partially-staged.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { jest } from '@jest/globals'

import { withGitIntegration } from './__utils__/withGitIntegration.js'
import * as fileFixtures from './__fixtures__/files.js'
import * as configFixtures from './__fixtures__/configs.js'

jest.setTimeout(20000)
jest.retryTimes(2)

describe('lint-staged', () => {
test(
'skips hiding unstaged changes from partially staged files with --no-hide-partially-staged',
withGitIntegration(async ({ execGit, gitCommit, readFile, writeFile }) => {
await writeFile('.lintstagedrc.json', JSON.stringify(configFixtures.prettierWrite))

// Stage ugly file
await writeFile('test.js', fileFixtures.uglyJS)
await execGit(['add', 'test.js'])

// modify file with unstaged changes
await writeFile('test.js', fileFixtures.uglyJSWithChanges)

// Run lint-staged with --no-hide-partially-staged
const stdout = await gitCommit({ lintStaged: { hidePartiallyStaged: false } })

expect(stdout).toMatch(
'Skipping hiding unstaged changes from partially staged files because `--no-hide-partially-staged` was used'
)

// Nothing is wrong, so a new commit is created
expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('2')
expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test')
expect(await readFile('test.js')).toEqual(fileFixtures.prettyJSWithChanges)
})
)
})
4 changes: 4 additions & 0 deletions test/integration/no-stash.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ describe('lint-staged', () => {
const stdout = await gitCommit({ lintStaged: { stash: false } })

expect(stdout).toMatch('Skipping backup because `--no-stash` was used')
expect(stdout).toMatch(
'Skipping hiding unstaged changes from partially staged files because `--no-stash` was used'
)

// Nothing is wrong, so a new commit is created
expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('2')
Expand All @@ -44,6 +47,7 @@ describe('lint-staged', () => {
gitCommit({
lintStaged: {
stash: false,
hidePartiallyStaged: true,
config: {
'*.js': async () => {
const testFile = path.join(cwd, 'test.js')
Expand Down