Skip to content

Commit

Permalink
feat: --no-stash flag implies --no-hide-partially-staged
Browse files Browse the repository at this point in the history
  • Loading branch information
giladgd authored and iiroj committed Dec 2, 2023
1 parent f4f61f3 commit f3378be
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/empty-gifts-leave.md
@@ -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
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
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
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
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
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
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
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
@@ -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
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

0 comments on commit f3378be

Please sign in to comment.