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.

Sign up for GitHub

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

Jump to bottom

Unstaged local changes get committed with --no-stash flag from version 15.2.0 #1468

Open
martinslota opened this issue Aug 28, 2024 · 6 comments
Open

Unstaged local changes get committed with --no-stash flag from version 15.2.0 #1468

martinslota opened this issue Aug 28, 2024 · 6 comments

Comments

@martinslota
Copy link

Description

Unstaged local changes get committed with --no-stash flag. This happens from version 15.2.0, and all the way up to the most recent version of lint-staged.

Expected: When unstaged changes exist within a file with staged changes, only the staged changes get committed.

Actual: Both unstaged and staged changes get committed.

Steps to reproduce

  1. Clone this repository and install dependencies using npm ci

  2. Create a new file and stage the changes:

    $ echo 'function test() {}' > index.js

$ git add index.js

  3. Make another change to the file with staged changes:

    $ echo 'function test2() {}' >> index.js

  4. Commit the changes:

    $ git commit -m "Adding test() without test2()"
⚠ Skipping backup because `--no-stash` was used.

⚠ Skipping hiding unstaged changes from partially staged files because `--no-stash` was used.

✔ Preparing lint-staged...
✔ Running tasks for staged files...
✔ Applying modifications from tasks...
[main 4a8ad81] Adding test() without test2()
 1 file changed, 2 insertions(+)
 create mode 100644 index.js

Debug Logs

expand to view 
❯ git commit -m "Adding test() without test2()"
  lint-staged:bin Running `[email protected]` on Node.js v20.15.0 (darwin) +0ms
  lint-staged:bin Options parsed from command-line: {
  allowEmpty: false,
  concurrent: true,
  configPath: undefined,
  cwd: undefined,
  debug: true,
  diff: undefined,
  diffFilter: undefined,
  maxArgLength: undefined,
  quiet: false,
  relative: false,
  shell: false,
  stash: false,
  hidePartiallyStaged: false,
  verbose: false
} +0ms
  lint-staged:validateOptions Validating options... +0ms
  lint-staged:validateOptions Validated options! +0ms
  lint-staged Unset GIT_LITERAL_PATHSPECS (was `undefined`) +0ms
  lint-staged:runAll Running all linter scripts... +0ms
  lint-staged:runAll Using working directory `/Users/martin.slota/Code/lint-staged-no-stash-bug` +0ms
  lint-staged:resolveGitRepo Resolving git repo from `/Users/martin.slota/Code/lint-staged-no-stash-bug` +0ms
  lint-staged:resolveGitRepo Unset GIT_DIR (was `undefined`) +0ms
  lint-staged:resolveGitRepo Unset GIT_WORK_TREE (was `undefined`) +0ms
  lint-staged:execGit Running git command [ 'rev-parse', '--show-prefix' ] +0ms
  lint-staged:resolveGitRepo Resolved git directory to be `/Users/martin.slota/Code/lint-staged-no-stash-bug` +13ms
  lint-staged:resolveGitRepo Resolved git config directory to be `/Users/martin.slota/Code/lint-staged-no-stash-bug/.git` +0ms
  lint-staged:execGit Running git command [ 'log', '-1' ] +13ms
⚠ Skipping backup because `--no-stash` was used.

⚠ Skipping hiding unstaged changes from partially staged files because `--no-stash` was used.

  lint-staged:execGit Running git command [ 'diff', '--name-only', '-z', '--diff-filter=ACMR', '--staged' ] +12ms
  lint-staged:runAll Loaded list of staged files in git:
  lint-staged:runAll [ '/Users/martin.slota/Code/lint-staged-no-stash-bug/index.js' ] +37ms
  lint-staged:searchConfigs Searching for configuration files... +0ms
  lint-staged:execGit Running git command [ 'ls-files', '-z', '--full-name' ] +12ms
  lint-staged:execGit Running git command [ 'ls-files', '-z', '--full-name', '--others', '--exclude-standard' ] +2ms
  lint-staged:searchConfigs Found possible config files: [ '/Users/martin.slota/Code/lint-staged-no-stash-bug/package.json' ] +12ms
  lint-staged:loadConfig Loading configuration from `/Users/martin.slota/Code/lint-staged-no-stash-bug/package.json`... +0ms
  lint-staged:loadConfig Successfully loaded config from `/Users/martin.slota/Code/lint-staged-no-stash-bug/package.json`:
  lint-staged:loadConfig { '*.js': [ 'prettier --check' ] } +0ms
  lint-staged:validateConfig Validating config from `/Users/martin.slota/Code/lint-staged-no-stash-bug/package.json`... +0ms
  lint-staged:validateConfig Validated config from `/Users/martin.slota/Code/lint-staged-no-stash-bug/package.json`: +1ms
  lint-staged:validateConfig {
  lint-staged:validateConfig   '*.js': [
  lint-staged:validateConfig     'prettier --check'
  lint-staged:validateConfig   ]
  lint-staged:validateConfig } +0ms
  lint-staged:searchConfigs Found 1 config files +1ms
  lint-staged:groupFilesByConfig Grouping 1 files by 1 configurations +0ms
  lint-staged:chunkFiles Resolved an argument string length of 58 characters from 1 files +0ms
  lint-staged:chunkFiles Creating 1 chunks for maxArgLength of 131072 +0ms
  lint-staged:generateTasks Generating linter tasks +0ms
  lint-staged:generateTasks Generated task: 
  lint-staged:generateTasks {
  lint-staged:generateTasks   pattern: '*.js',
  lint-staged:generateTasks   commands: [ 'prettier --check' ],
  lint-staged:generateTasks   fileList: [ '/Users/martin.slota/Code/lint-staged-no-stash-bug/index.js' ]
  lint-staged:generateTasks } +1ms
  lint-staged:makeCmdTasks Creating listr tasks for commands [ 'prettier --check' ] +0ms
  lint-staged:resolveTaskFn cmd: prettier +0ms
  lint-staged:resolveTaskFn args: [ '--check' ] +0ms
  lint-staged:resolveTaskFn execaOptions: {
  cwd: '/Users/martin.slota/Code/lint-staged-no-stash-bug',
  preferLocal: true,
  reject: false,
  shell: false
} +0ms
  lint-staged:chunkFiles Resolved an argument string length of 58 characters from 1 files +1ms
  lint-staged:chunkFiles Creating 1 chunks for maxArgLength of 131072 +0ms
[STARTED] Preparing lint-staged...
  lint-staged:GitWorkflow Backing up original state... +0ms
  lint-staged:GitWorkflow Getting partially staged files... +0ms
  lint-staged:execGit Running git command [ 'status', '-z' ] +14ms
  lint-staged:GitWorkflow Found partially staged files: [ 'index.js' ] +17ms
  lint-staged:execGit Running git command [
  'diff',
  '--binary',
  '--unified=0',
  '--no-color',
  '--no-ext-diff',
  '--src-prefix=a/',
  '--dst-prefix=b/',
  '--patch',
  '--submodule=short',
  '--output',
  '/Users/martin.slota/Code/lint-staged-no-stash-bug/.git/lint-staged_unstaged.patch',
  '--',
  'index.js'
] +17ms
[COMPLETED] Preparing lint-staged...
[STARTED] Running tasks for staged files...
[STARTED] package.json — 1 file
[STARTED] *.js — 1 file
[STARTED] prettier --check
[COMPLETED] prettier --check
[COMPLETED] *.js — 1 file
[COMPLETED] package.json — 1 file
[COMPLETED] Running tasks for staged files...
[STARTED] Applying modifications from tasks...
  lint-staged:GitWorkflow Adding task modifications to index... +127ms
  lint-staged:execGit Running git command [
  'add',
  '--',
  '/Users/martin.slota/Code/lint-staged-no-stash-bug/index.js'
] +127ms
  lint-staged:GitWorkflow Done adding task modifications to index! +11ms
  lint-staged:execGit Running git command [ 'diff', '--name-only', '-z', '--diff-filter=ACMR', '--staged' ] +11ms
[COMPLETED] Applying modifications from tasks...
  lint-staged Tasks were executed successfully! +220ms
[main e395f12] Adding test() without test2()
 1 file changed, 2 insertions(+)
 create mode 100644 index.js

Environment

  • OS: MacOS Sonoma
  • Node.js: v20.15.0
  • lint-staged: 15.2.0
@iiroj
Copy link
Member

iiroj commented Aug 29, 2024 •
edited
Loading

The unstaged changes are normally put into the stash, which gets disabled with --no-stash. So they can only be discarded or staged.

Why are you using the option in this case?

@martinslota
Copy link
Author

The unstaged changes are normally put into the stash, which gets disabled with --no-stash. So they can only be discarded or staged.

Why are you using the option in this case?

We are using —no-stash because without it the commits are a lot slower - by as much as 50%.

The option used to be compatible with having files with some staged and some unstaged changes. Having all of the changes committed is quite confusing.

@iiroj
Copy link
Member

iiroj commented Aug 29, 2024

If --no-stash didn't add unstaged changes, it would break the expected behavior of staging linter scripts' edits (for example prettier --write).

I don't want to change this behavior because both have their pros and cons.

One possibility might to add a flag --no-add that would achieve this when combined with --no-stash; it has been requested before.

@iiroj
Copy link
Member

iiroj commented Aug 29, 2024

Ref #1262

@martinslota
Copy link
Author

If --no-stash didn't add unstaged changes, it would break the expected behavior of staging linter scripts' edits (for example prettier --write).

Thank you, that clarifies things quite a bit. 🙇

I respectfully disagree that adding unstaged changes is a good default for the following reasons:

  1. It seems like --no-stash did not add unstaged changes before version 15.2.0, and changing that is a breaking change - if for no one else, then at least for my colleagues and myself.

  2. It goes against the behaviour that git commit has on its own, so from that perspective it seems quite surprising.

  3. It goes against the behaviour that lint-staged has without the --no-stash flag - and while using the Git stash to implement some of the features in lint-staged sounds fine, the core feature of checking staged changes and committing them if the checks pass does not seem to require using the Git stash.

I don't want to change this behavior because both have their pros and cons.

The way I understand the situation, the behaviour has recently been changed. But I suppose you are referring to reverting that change and going back to the behaviour in versions 15.1.0 and earlier.

One possibility might to add a flag --no-add that would achieve this when combined with --no-stash; it has been requested before.

That does sound like a pretty good workaround. 👍 I will vote in favour of the referenced issue.

In case I haven't managed to change your mind, I think it makes sense to close this issue now.

@gunhoflash
Copy link

I solved it by using --hide-partially-staged since --no-stash implies --no-hide-partially-staged.

lint-staged --no-stash --hide-partially-staged

referred: #1294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gunhoflash @iiroj @martinslota