Lint-staged: Add better support for partially staged files

[sedubois]

See https://github.com/mxstbr/react-boilerplate/issues/1064#issuecomment-251912084 for description

[okonet]

Partial staging bugging me a lot but I don't know how to solve this. If you know how to solve that issue, you're welcome to re-open the issue and file a PR. Otherwise I'll close the issue as won't fix.

[sedubois]

Sure 😊 I'm not familiar with the mechanism, so I don't have a solution to propose right now.

[sedubois]

Why not simply temporarily doing git stash -k?

[okonet]

Can you elaborate on ↑ since I'm not familiar with how it works.

This sounds interesting and is worth trying. Thanks for pointing it out.

[okonet]

@sedubois I'm a bit stuck with #75. Do you have ideas how to solve it probably?

[sedubois]

Hmm. Maybe first concentrate on the simple case when linter doesn't modify the working copy, and address the case of eslint --fix separately?

Also you proposed in PR to git stash pop after the commit, but conflicts would also happen in that case, no? Sounds like the conflict is unavoidable and the user should then be given the opportunity to resolve it?

[okonet]

Also you proposed in PR to git stash pop after the commit, but conflicts would also happen in that case, no?

I don't think it will. The conflict will only happen if the actual content has diverged from stashed one. Otherwise, it will successfully merge them. So this use case can be solved with the post-command.

address the case of eslint --fix separately

I really don't want to loose this feature. So if there is a way of not losing it, I'd take this effort. So for now this is the only issue that prevents me from releasing it (besides of some code that needs to be written and the cleanup).

[okonet]

For now, the second case doesn't seem to be solvable without a user interaction which sucks :(

If someone has a better solution for that, any ideas are appreciated!

See #73 for some more context.

[sedubois]

Well, if you stash then allow linters to perform changes, conflicts might happen. Either don't stash, or disable the linter --fix option. Maybe you can let the user choose what they prefer.

Personally I would just disable the --fix option, so that I can use stash -k. Otherwise, lint-staged's behavior is simply incorrect (it lints code which isn't meant to be committed).

[okonet]

These aren't solutions but workaround of the problem :(

To add git stash you don't need to change anything in the library itself. You can just add those to the config:

{
  "*.js": ["git stash -k", "eslint", "git stash pop"]
}

[sedubois]

Oh that's nice, I'll give that a try. Should it be documented?

But how is that a problem? Isn't it normal to ask for user input, if a merge conflict occurs?

[okonet]

Well, technically, yes :) But in this case, I don't think so since the changes are coming from eslint and this will result in annoying user prompts.

BTW: I agree this should be backed into the library but the --fix case is just too important to me.

[sedubois]

Actually, didn't manage to get this working:

 ❯ Running tasks for *.js
   →    or: git stash clear
   ✖ git stash -k
     →    or: git stash clear
     eslint
     git stash pop

🚫  git stash -k found some errors. Please fix them and try committing again.

usage: git stash list [<options>]
   or: git stash show [<stash>]
   or: git stash drop [-q|--quiet] [<stash>]
   or: git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]
   or: git stash branch <branchname> [<stash>]
   or: git stash [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
                       [-u|--include-untracked] [-a|--all] [<message>]]
   or: git stash clear



pre-commit: 
pre-commit: We've failed to pass the specified git pre-commit hooks as the `lint-staged`
pre-commit: hook returned an exit code (1). If you're feeling adventurous you can
pre-commit: skip the git pre-commit hooks by adding the following flags to your commit:
pre-commit: 
pre-commit:   git commit -n (or --no-verify)
pre-commit: 
pre-commit: This is ill-advised since the commit is broken.
pre-commit: 

[okonet]

Hmm, it's probably since I'm always passing paths to the command 🤔

[kmiyashiro]

The readme mentions that as of 3.1, lint-staged stashes un-staged changes. This is apparently not the case?

[okonet]

@kmiyashiro yes, it seems it slipped into the Readme. I'll remove it.

[leonaves]

So, while I don't know what the approach would be for getting this working in lint-staged, I think conflicts are always going to be an issue, i managed to get this working using pre-commit and some git trickery, with a lot owed to this answer on stack overflow.

My partial package.json:

  "lint-staged": {
    "*.js": [
      "eslint",
      "jest --bail --findRelatedTests"
    ],
    "*.scss": "stylelint --syntax scss",
    "*.css": "stylelint"
  },
  "scripts": {
    "lint-staged": "lint-staged || (yarn run pop-stash >> /dev/null && exit 1)",
    "stash-unstaged": "git stash save -k 'pre-linting-stash' >> /dev/null",
    "pop-stash": "git stash && git stash pop stash@{1} && git read-tree stash && git stash drop",
  },
  "pre-commit": [
    "stash-unstaged",
    "lint-staged",
    "pop-stash"
  ],

This works by first stashing the unstaged hunks, as above, then running the lint-staged script, which will pop the stash if it fails, and return a non-zero exit code to ensure it fails the overall pre-commit hook. Lastly it uses the trick in the aforelinked stackoverflow answer to safely return the stashed changes.

Hope this helps someone!

[okonet]

@leonaves that's awesome and I'm gonna try it out. Did you see @75? This is pretty much the same what I was trying to implement but I could not come up with the right command to stash pop.

Also, how does this work in case of aborted commit?

Does it also works with updating the code (eslint --fix)?

[leonaves]

@okonet Ah cool, yeah that stack overflow answer really helped me, it's a clever sequence of git commands, I don't think I would got there myself in the end.

With regards to the fix flag, I've been playing with it this morning, and it's actually a really tough problem, that I'm not sure will be possible cleanly or without some user interaction.

Say I have the following code:

export default function() {
  return "My hello world module";
}

I stage this and then add the following line:

export default function() {
  console.log("I don't want this committed just yet");
  return "My hello world module";
}

If I run the steps from my previous comment with eslint --fix and git add . as well, what happens is this:

• The unstaged hunk is stashed, leaving us with this again:

export default function() {
  return "My hello world module";
}

• eslint --fix automaticaly fixes the double string issue, leaving us with this:

export default function() {
  return 'My hello world module';
}

• git add . then stages that change as well, so that the above fixed file in its entirety is in the index.
• We stash the current index, leaving our working tree clean.
• We apply the previously unstaged changes from the stash stack, giving us this unstaged:

export default function() {
  console.log("I don't want this committed just yet");
  return "My hello world module";
}

• We read from the top of the stash into the index, giving us this in the index:

export default function() {
  return 'My hello world module';
}

• That will be committed, which is correct, however once it's committed, we will have a dirty working tree with the following:

export default function() {
  console.log("I don't want this committed just yet");
  return "My hello world module"; // This fix has been undone.
}

I don't think the end result is achievable, without also applying a linting fix to the console.log line, which to be honest, would probably be acceptable to pretty much everyone, as long as that line is not added to the index. But you would have to do some inefficient hoop jumping to do so:

• Before stashing anything, apply eslint --fix and swallow the exit code (we don't want this aborting the commit if it fails as unstaged hunks might have unfixable linting errors that aren't the problem of the current commit).
• Stash the unstaged changes (this would stash all the fixes just applied to the working tree).
• Run eslint --fix again, this time not swallowing the exit code (unfixable errors in the index should abort the commit).
• Stash the entire working tree + index.
• Apply the first stash onto the working tree with git stash apply @{1}.
• Read the second stash into the index with git read-tree stash.
• Drop the last stash on the stash stack.

Now your working tree should contain all of the same fixes as your index, as we applied eslint --fix before stashing the first time, and it will also have some fixes of it's own, albeit unstaged. These can then be either checked out or applied to the next commit. The main downside I see to this is that we are running eslint twice, but so far I can't see another way to do it, again, unless you apply the stashes to the working tree and have the user deal with conflicts.

Anyway, sorry this turned out so long, but hope it helps.

[AndersDJohnson]

Also consider adding the the -u flag to git stash save -k so we also stash any untracked files such that they don't cause any lint failures unrelated to the staged files. Refer to: https://git-scm.com/docs/git-stash#git-stash-save-p--patch-k--no-keep-index-u--include-untracked-a--all-q--quietltmessagegt.

[okonet]

How would they if we only lint staged files?

[AndersDJohnson]

@okonet True, in most cases untracked files wouldn't be picked up, unless the scripts being run don't support accepting a list of files as arguments, instead running on the entire directory anyway.

[leonaves]

As a heads up, I've been using this technique for a few days now, and for some reason the read-tree command fails after a merge with conflicts with the following error:

fatal: could not open '.git/MERGE_HEAD' for reading: No such file or directory

Running git commit a second time makes the commit work, not sure why yet.

[okonet]

@leonaves thanks for the heads up! I'm playing with it in the https://github.com/okonet/lint-staged/tree/git-worflow branch so feel free to check it out and help.

[asottile]

http://pre-commit.com handles this (and with fixers) - - you may find the strategy used there useful.

[okonet]

Interesting! Will check it out but it seems they also doesn't manage conflicts from autofixes.

https://github.com/pre-commit/pre-commit/blob/master/pre_commit/staged_files_only.py

[asottile]

It does: https://github.com/pre-commit/pre-commit/blob/1be4e4f82e31336fa5fca096c962c72ac0041537/pre_commit/staged_files_only.py#L50-L58 -- on conflict, the autofixes are rolled back and the unstaged changes are re-applied

[asottile]

Disclaimer: I wrote and maintain pre-commit/pre-commit -- if you have any questions I'd be happy to answer :)

[okonet]

Hey @asottile! I'll definitely ping you for some help. I was referring to the problem that you'll remove the fixes but I see now why it is a better solution. Thanks for chiming in. Please take a look at the git workflow branch if you have time and let me know what you think.

[Stupidism]

@okonet I fixed some problems in @leonaves 's code in my project. I put a PR in react-starter-kit.

Problems

Problems fixed:

• [x] when there's nothing to commit and your stash list is not clear, pos-stash would pop a stash you don't want
• [ ] when you press Ctrl+C during commit, pop-stash would not pop for you, you can only manually do that
• [x] when there's nothing to commit after lint-staged, commit should be stopped

Solution with npm scripts and bash scripts

  "scripts": {
    "stash-unstaged": "git stash save -k --include-untracked 'unstaged-stash' >> /dev/null",
    "pop-if-gst-clean": "./tools/pop-if-gst-clean.sh",
    "lint-staged": "lint-staged || (npm run pop-stash >> /dev/null && exit 1)",
    "pop-if-gd-clean": "./tools/pop-if-gd-clean.sh",
    "stash-linted": "git stash save 'linted-stash' >> /dev/null",
    "stash-pop-unstaged": "git stash pop stash@{1}",
    "stash-pop-linted": "git read-tree stash && git stash drop",
    "clean-lint-staged": "lint-staged >> /dev/null"
  },
  "//pre-commit": {
    "//reference-links": {
      "github": "https://github.com/okonet/lint-staged/issues/62#issuecomment-286830310",
      "stackoverflow": "http://stackoverflow.com/questions/13889242/26685296#26685296"
    },
    "//stash-pop-linted": "use `git read-tree` to resolve conflicts",
    "//clean-lint-staged": [
      "Re-run `lint-staged` for fixable style errors mistakenly recovered to be unstaged by `read-tree`",
      "NOTICE: Although they are fixed, you will see them when input a commit-message",
      "Side effect is it will auto-fix unstaged part in these staged files",
      "Feel free to remove this cmd if you don't want the side effect or just feel adventurous",
      "If so, don't be surprised when you see unstaged&fixable style errors after commit"
    ]
  },
  "pre-commit": [
    "stash-unstaged",
    "pop-if-gst-clean",
    "lint-staged",
    "pop-if-gd-clean",
    "stash-linted",
    "stash-pop-unstaged",
    "stash-pop-linted",
    "clean-lint-staged"
  ]

Help wanted to make it into lint-staged

But like they said,

the solution is too complex to be used in multiple projects and also contains cross-platform issues like does not work on windows

I'm not familiar with cross-platform issues. Can you fulfill this in this repo?

[asottile]

fwiw in my project I avoid git stash due to all the edge cases and its
tendency to interfere with people who actually use git stash (which my
phone strongly believes should be auto corrected to git Satan). The biggest
problem being that git stash can lose information

On May 5, 2017 10:23 AM, "Stupid" [e-mail] wrote:

@okonet https://github.com/okonet I fixed some problems in @leonaves
https://github.com/leonaves 's code in my project
https://github.com/Stupidism/react-starter-kit/commit/17066dbdd6454569fb3e81787eb97f39b9888653.
I put a PR in react-starter-kit
https://github.com/kriasoft/react-starter-kit/pull/1230/files.
Problems

Problems fixed:

• when there's nothing to commit and your stash list is not clear,
  pos-stash would pop a stash you don't want
• when you press Ctrl+C during commit, pop-stash would not pop for
  you, you can only manually do that
• when there's nothing to commit after lint-staged, commit should be
  stopped

Help Wanted

But like they said
https://github.com/kriasoft/react-starter-kit/pull/1230#issuecomment-299384234
,

the solution is too complex to be used in multiple projects and also
contains cross-platform issues like does not work on windows

I'm not familiar with cross-platform issues. Can you fulfill this in this
repo?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/okonet/lint-staged/issues/62#issuecomment-299525019,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABugn4rVz8tyLbCWNqTch6PM7lMFEFjaks5r21slgaJpZM4KPwFp
.

[Stupidism]

@asottile I didn't find any info loss in this solution(not mean git stash) until now. Can you name it more specificly?
Considering it's just partially stash and stash pop, I think it's quite safe.

[asottile]

Notably:

• git stash + conflicts
• git stash + ^C
• git stash + ^

popping a stash which conflicts causes the stash to be dropped and the changes evaporating to the aether

[Stupidism]

@asottile I think we should consider it in this lint partially stage situation

• git stash + lint + pop => conflicts -- not gona happen using git read-tree
• git stash + lint + ^C -- use must run git stash pop by hand
• git stash + ^ -- I don't understand this one...

[asottile]

^ sends SIGQUIT (it's like ^C but with moar power)

SIGINT (^C) is catchable so you should really try to undo things when it happens
SIGQUIT (^) is unavoidable in either case so it's really the user's fault if they use this heavy of a sledge hammer (but at least it won't clobber their private state)

pre-commit avoids this by maintaining its own patch file separate from the (full of spiders) git stash stack

[motiz88]

There's one low-hanging fruit I can see here, in the form of automatically restaging fully-staged files after --fixing them. Assuming most files in most commits are fully staged, this is a significant win - one only ever needs to manually amend a commit if there's a lint fix in a partially staged file.

With this in mind I created git-exec-and-restage, which can be configured in package.json as seen below. However, if there's any interest in building it directly into lint-staged (behind a flag?) I'd love to put together a PR.

{
  "scripts": {
    "precommit": "lint-staged"
  },
  "lint-staged": {
    "*.js": ["git-exec-and-restage eslint --fix --"]
  }
}

[okonet]

@motiz88 I think this is amazing! I'd love to integrate this into the core to make the setup even simpler. I think this could be the default behavior. Thoughts?

[Stupidism]

@okonet Still can't do partially commit and fix well

@motiz88 what's the equivalent cmds of your cmd git-exec-and-restage eslint --fix --?

[motiz88]

@Stupidism

git-exec-and-restage eslint --fix -- fullystaged.js partiallystaged.js would run the following:

1. git diff --name-only --diff-filter=ACDMRTUXB fullystaged.js partiallystaged.js (any files returned by this command are not fully staged, so will be excluded in step 3)
2. eslint --fix fullystaged.js partiallystaged.js (run the requested command on all files)
3. git add fullystaged.js (add only the files that started out as fully staged)

Have a look at the test suite - The corresponding snapshots show exactly which commands are run in each case.

[MoOx]

Thanks for this solution @motiz88, it helps a lot!

[benjie]

I got it working with:

  scripts: {
    // ...
    "precommit": "lint-staged",
    "lint-staged-stash": "git stash save --keep-index 'lint-staged' && touch .didstash || rm .didstash || true",
    "lint-staged-stash-pop": "test -f .didstash && rm .didstash && git stash pop || true"
  },
  "lint-staged": {
    "*.js": ["lint-staged-stash", "eslint --fix", "git add", "lint-staged-stash-pop"]
  },

but I think I'm going simplify and disable auto-fixing, assuming people have it in their editor already - just throw an error on lint fail.

[Stupidism]

@benjie I think conflicts would occur if file changed by eslint --fix

[benjie]

@Stupidism Yes, they do in rare cases though they're generally easy to resolve from my testing. Nonetheless I use patch adding enough that I'm just going to use lint-staged as a warning rather than automatically adding the fixes - I like to know exactly what's going into my git history!

What I really want is something like eslint --fix=exit-nonzero where if any fixes occur it does them (and writes them out) but exits with non-zero exit code so that I then have to add the changes manually and commit again. Is anyone aware of something like this?

[okonet]

One idea I had in my hard is to have a postcommit hook with fixing and creating a fixup commit. But I don't know how to do this in a reliable way. Does anyone want to explore this?

[asottile]

@benjie http://pre-commit.com does exactly that :)

[JLHwung]

Inspired from @motiz88 's git-exec-and-restage, could we add a fullyStagedOnly: boolean flag to the configuration so that

1. when fullyStagedOnly is true, run lint only on fully staged files specified by linters
2. when fullyStagedOnly is false, no extra behavior is introduced. This is the default behavior.

As prettier does not (and will not ever I think) support linting chunks, partially staged files will always be a headache for lint-staged on a prettier + git add workflow. If fullyStagedOnly is supported, prettier + git add will become secure since git add would never add unstaged changes.

The drawback of fullStagedOnly will be the case when a dev can commit partially staged files and then discard local changes to circumvent lint-staged. Anyway the dev could pass any git hooks by git commit -n so I don't think it will hinder this fullStagedOnly workflow.

Further discussion is definitely welcomed.

[SimenB]

As prettier does not (and will not ever I think) support linting chunks

It supports ranges today.

https://github.com/prettier/prettier/blob/3f6a232cea56f60f1ba50cbae2d56f78b36a7699/README.md#range

[JLHwung]

It supports ranges today.
https://github.com/prettier/prettier/blob/3f6a232cea56f60f1ba50cbae2d56f78b36a7699/README.md#range

Good catch!

It seems that the range option is mostly suited for editor integration. If one uses range to lint partially staged files, prettier is expected to return the formatted line numbers for git partially staging. Otherwise git add will still add unstaged hunks.

[okonet]

@JLHwung I'm wondering if I could just integrate git-exec-and-restage and is it by default, what would be the drawbacks?

[JLHwung]

@okonet If git-exec-and-restage is enabled by default. There will be a case where devs will NOT have a staged file linted and they might be even unaware of that.

For example, say we have a.js and b.js revised. Say b.js is revised between line 118-128 and line 138-148. The first time we stage a.js and line 118 - 128 of b.js and the linter will run on a.js only. Now the second time we find the line 138 - 148 is error-prone so we decide simply discard the changes between line 138-148. In this case, b.js is never linted and the only way to trigger the linter is revised b.js again.

Other than this case where a partially staged file become actually fully staged via the succeeding operations, it seems git-exec-and-restage can be a safe default. Anyway it is a breaking change and should be well documented if enabled.

[okonet]

@JLHwung I'm not sure I completely understood your example, TBH. I believe git-exec-and-restage will run on both files but if you stage just a hunk of b.js, the second change will be discarded during the run of linter, so you'll lint/process only this hunk. After it's done, git-exec-and-restage will revert the second change in b.js so when you run pre-commit on it, it will be linted again. What am I missing?

It will be a breaking change for sure!

[JLHwung]

@okonet The workflow of git-exec-and-restage is stated here. I will explain my example in details. Now we use the fullystaged.js and partiallystaged.js as example. Say there is two hunk, namely 1 and 2 of partiallystaged.js.

So what will happen after we run

git add fullystaged.js
git add -p partiallystaged.js # y for first hunk and n for the next hunk
git commit

The pre-commit hook runs as:

1. git-exec-and-restage finds out that both fullystaged.js and partiallystaged.js is staged in this commit.
2. run linter/formatter on both fullystaged.js and partiallystaged.js
3. run git add fullystaged.js

After the hook runs, git commit will continue.

In this example, the final commit HEAD will include:

a. a formatted version of fullystaged.js
b. a non-formatted version of partiallystaged.js

The working directory will include:

a. a formatted version of partiallystaged.js

If we later fully stage partiallystaged.js, everything is good and well formatted after we commit the fully-staged partiallystaged.js.

However, if we run git checkout -- partiallystaged.js, the git repository will have partiallystaged.js non formatted and never format unless we later revise partiallystaged.js and fully stage.

The lump-sum result is devs can have a unformatted version of partiallystaged.js in some commit. But like I said before, they can also circumvent pre-commit hook by git commit -n.

As pre-commit is never 💩-proof since git commit -n, I think it is acceptable to align lint-staged with git-exec-and-restage workflow.

[julienw]

What I really want is something like eslint --fix=exit-nonzero where if any fixes occur it does them (and writes them out) but exits with non-zero exit code so that I then have to add the changes manually and commit again. Is anyone aware of something like this?

@benjie suggestion: run eslint first, if this fails run eslint --fix and exit with an error code :)

[benjie]

@julienw Thanks for the suggestion, but that'l double the lint time which is already too long :(

[julienw]

Thanks to lint-staged, it's not so long, but yeah I quite missed this would double the time.

[benjie]

I'm considering using eslint_d or something to make it faster but I've not got around to it yet. Anyway this is off-topic so I'll shut up :)

[niedzielski]

suggestion: run eslint first, if this fails run eslint --fix and exit with an error code :)

Enabling caching with the --cache option may improve performance but probably not adequately.

[julienw]

To recap, the issue here is when we automatically run git add on files that the developer only partially staged, the direct result being that the full file is committed.

I got an idea: make it possible to configure different commands for partially staged vs fully staged files.

For example, partially staged would have only eslint being run while fully staged would have eslint --fix, git add.

Thoughts ?

[okonet]

@julienw this is feasible but not the best UX IMO. Ideally, developers should not think about commands at all and this should be implementation detail of lint-staged. But I still don't have a solution to the problem that would cover all edge cases.

My concern about your idea is how the config will look like. Ideas?

[julienw]

I'm not entirely sure, but I think the config format will change in #273 anyway so this could be included in the format overhaul. I haven't studied how the proposed format looks like yet.

It's because there are a lot of edge cases that I think this idea is good: it's much more conservative, doesn't try to be clever, but still provides a service to the developer by deciding whether a file is partially or fully staged.

[benjie]

One option is to check out the staged files into a separate folder and lint fix them and add back into the index then commit.

[okonet]

@benjie yes, this is what I'd like to try next TBH. If you have time to work on it, you're welcome to draft a PR!

[userUnderC]

I'm new to this but I'm wondering if this may be related to something I've been seeing.

I work in pycharm and have lint-staged piping to prettier. What will happen to me, is I will commit and push and that workd - and then my committed files will show up as uncommitted but have no changes.

I wonder if git-exec-and-restage would help with that?

[julienw]

One option is to check out the staged files into a separate folder and lint fix them and add back into the index then commit.

too magical for my taste... but maybe if this works perfectly I won't even see it. ;)

@userUnderC this issue is only about partially staged files (using git add -p). I'd say your problem is something else unrelated to this issue.

[okonet]

@userUnderC you're probably hitting this #151

[julien-f]

Here is my current approach, it works quite well except that the files are not unstashed on errors:

{
  "scripts": {
    "lint-staged-stash": "git diff > .git/lint-staged.diff && git checkout . && true",
    "lint-staged-unstash": "patch < .git/lint-staged.diff && rm -f .git/lint-staged.diff && true",
    "precommit": "lint-staged"
  },
  "lint-staged": {
    "*.js": [
      "lint-staged-stash",
      "eslint --fix",
      "jest --findRelatedTests",
      "git add",
      "lint-staged-unstash"
    ]
  }
}

[okonet]

@julien-f do you think if we would incorporate this approach to the lib and do unstash in case of error would solve it? Do you mind creating a PR so I can test it?

[asottile]

@okonet @julien-f having some experience with that strategy, it does work well, you'll want to instead use the following commands (more robust):

from here

git diff-index --ignore-submodules --binary --exit-code --no-color --no-ext-diff $tree --

and for apply (from here)

git apply --whitespace=nowarn patch ||
    git -c core.autocrlf=false apply --whitespace=nowarn patch

If you want a testsuite for this, here's a pretty comprehensive one: https://github.com/pre-commit/pre-commit/blob/master/tests/staged_files_only_test.py

[alexilyaev]

My colleagues and I had several times that unstaged files got committed, so I've decided to try out the solutions @julien-f and @asottile suggested.

Notes:

• The && true at the end is important, since lint-staged adds the staged files paths at the end of the given command (30 min wasted cause I was a smart-ass and removed those, bah).
• git-apply seems to be better than patch
  (ref https://www.lullabot.com/articles/git-best-practices-upgrading-the-patch-process)
  > apply has two key differences from patch. First, it will not apply a patch if you have other uncommitted changes in your code. The other significant difference is that by default, git apply will not apply a patch that does not apply cleanly...
• If we have several glob patterns, we need to add "concurrent": false, and put the settings under "linters" (see example below)

Questions @asottile

• How do I use git diff-index? What do I put in place of $tree?
  I couldn't get it to work
• Why git diff-index vs git diff?
• Why --exit-code? It exits with 1 if there is a difference
• Do we really need --no-ext-diff?
• In what case we'd need git -c core.autocrlf=false before the apply?

Working setup example

  "scripts": {
    "lint-staged-stash": "git diff --ignore-submodules --binary --no-color > lint-staged.diff && git checkout . && true",
    "lint-staged-unstash": "git apply --whitespace=nowarn lint-staged.diff && rm -f lint-staged.diff && true",
    "precommit": "lint-staged"
  },
  "lint-staged": {
    "concurrent": false,
    "linters": {
      "*.js": [
        "lint-staged-stash",
        "eslint --fix",
        "prettier --write",
        "git add",
        "lint-staged-unstash"
      ],
      "*.scss": [
        "lint-staged-stash",
        "stylelint --syntax scss --fix",
        "git add",
        "lint-staged-unstash"
      ]
    }
  },

This worked, and already much much better than silently adding and committing unstaged files.
If lint-staged did this behind the scenes and just documented how to apply the patch on failure, it would be great. But as @okonet mentioned, it would be possible to catch the error and apply the patch any way.

[asottile]

How do I use git diff-index? What do I put in place of $tree?
I couldn't get it to work

here's where I set up tree

Why git diff-index vs git diff?

git diff is a porcelain command whereas git diff-index is a plumbing command. Generally tools should only use plumbing commands. more on this

Why --exit-code? It exits with 1 if there is a difference

This was changed in a more recent version of git, previous versions did not do this without --exit-code :( -- then again, if you don't care about the exit code then this doesn't matter

Do we really need --no-ext-diff?

Say a user is using vimdiff, git apply isn't going to necessarily understand their patch output. A simpler case, consider their external diff provider is just diff --color boom

In what case we'd need git -c core.autocrlf=false before the apply?

the code comment covers this. The specific issue in question: https://github.com/pre-commit/pre-commit/issues/570

[alexilyaev]

@asottile Thanks!

So that worked:

git diff-index --ignore-submodules --binary --exit-code --no-color --no-ext-diff $(git write-tree) > lint-staged.diff

Patch output is the same, but I guess it has it's benefits over git diff.

Interesting that every flag has a story :-).

Issue with empty patch file

If there are no unstaged files, the patch will be empty, is there an easy way to not run the apply if the file is empty? I found [ -s lint-staged.diff ] returns 1 or 0 but this didn't work:

[ -s lint-staged.diff ] && git apply --whitespace=nowarn lint-staged.diff && rm -f lint-staged.diff && true

This is getting real messy and complex, definitely should be handled internally.

[asottile]

lol, at this point it's getting close to "derivative work"

fwiw, here's how pre-commit handles the empty patch: https://github.com/pre-commit/pre-commit/blob/41d998f1c46f371c9977dcc9d31d7b42387ed74c/pre_commit/staged_files_only.py#L39

(and yes, there are cases where git has a returncode indicating there's a diff but also produces an empty patch)

[tilgovi]

I'm not sure there's a way I can see this working with --fix unless the fix is applied before the stashing so that the stash is a diff against what will be committed rather than something that was changed before committing.

[alexilyaev]

@asottile Hmm, I'm trying to solve this as a shell command for now.
I'm just missing the proper conditional with && or || or something.
Any idea?

@tilgovi The goal is to stash unstaged files so that git add would not stage them before commit.
This means that --fix can't run before the stash, since then we'll have mixed pre-fix unstaged files and post-fix unstaged files.

[tilgovi]

My impression is that this issue is about partially staged files, where the stash patch fails to apply after the fix because the partially staged files have changed.

It seems fine to me to stash unstaged files to avoid making changes to them with a linter, but if a file is partially staged I have little hope that it can have its unstaged hunks stashed before the linting and still there be an expectation that unstashing will apply cleanly.

How much of this issue is solved if we accepted that even unstaged hunks of partially staged files will be linted?

[julienw]

@tilgovi I don't find a way your suggestion could work. I don't see how you would add the fixes for only the staged hunks, which is a goal. Then the staged and commited hunks won't have the fixes, as they're already in the index and we're not adding them again. And because --fix doesn't error if it can change the errors, you wouldn't warned about it.

I still think we shouldn't be too clever and not try to fix in case of partially staged hunks. But we need some support from lint-staged so that we can define separate commands for completely staged and partially staged files.

[okonet]

I'm currently actively working on the implementation for it in https://github.com/okonet/lint-staged/pull/75 and looks like my tests just got passed so there is some hope. I'll need to add more tests, though.

The solution I might end up will be a combination of all mentioned approaches.

[okonet]

As @julienw mentioned

I don't see how you would add the fixes for only the staged hunks, which is a goal

I'm running into this problem in my tests. It doesn't look like it's solvable by using files, which we're limited to.

@asottile I'm borrowing your code ATM and it seems like you not even trying to solve this case. Is this for the reason I mentioned above?

[asottile]

yeah, pre-commit handles both of the issues listed above, the entire implementation of this is here

I mentioned more on this back in april in this thread

[okonet]

So I'm back to the point there I need to meet a decision of what the expected behavior in case of partially staged files would be. Suggestions?

[okonet]

@asottile I'm confused. How do you handle the case there I have a partially staged file and I'd like to only apply fixes to staged hunks? The code you're referencing saying you just rolling back to the initial patch: https://github.com/pre-commit/pre-commit/blob/41d998f1c46f371c9977dcc9d31d7b42387ed74c/pre_commit/staged_files_only.py#L64-L66

[asottile]

pre-commit does:

• stash changes
• run hooks
• apply stashed changes
  
  
  
  • if this fails (conflicts): roll back hook modifications and retry applying stashed changes
  
  

[okonet]

I've just written a test for what I'd like to achieve: https://github.com/okonet/lint-staged/pull/75/commits/fc9a2ae319879c00dcfcc9ec447a16011ac08cd6 could you please take a look a confirm that pre-commit handles that?

[okonet]

@asottile I've borrowed this logic and expanded it to use @Stupidism ideas. Here is the test suite I'm running it against: https://github.com/okonet/lint-staged/pull/75/files#diff-f5002743e0b16c70d5bc43040f1d3847

[asottile]

The main difference is lint-staged runs git add without prompting. pre-commit takes a more timid stance on this (hooks after all are written by humans and therefore can have mistakes) and leaves it up to the user to ensure changes made by hooks are sane and committable. As a user, I'd be quite surprised if a tool was running git add on my behalf :)

EDIT: To elaborate on that, consider the worst case: a malicious hook which writes modifications and gets automatically added to the commit :)

[okonet]

@asottile from your answer I still missing if pre-commit is handling the case I mentioned. With or without user input — that doesn't matter in this case, does it?

[asottile]

Yes it handles the case, if stashed changes conflict with hook fixes, the stashed changes win and the repository state is put back to where it was when the commit started.

[okonet]

@asottile the case I mentioning is a different one. What I'm trying to do is this

1. stash local changes
2. run hooks with fixes
3. if all hooks return 0, add fixes to index
4. apply the local changes back

This is covered by this test: https://github.com/okonet/lint-staged/pull/75/files#diff-f5002743e0b16c70d5bc43040f1d3847R257

Now I'd like to do the same but with partially staged file. This is where it gets more complicated and it seems to be a no reliable way to implement it. Which makes me think if a compromise solution should be accepted in this case: for example, I could fail the whole hook in that case and say that fixes can't be applied to a partially staged file and leave it up to you.

Does anyone think that would be acceptable?

[asottile]

pre-commit takes the stance that 3. above is unacceptable and surprising :)

[renke]

I'd love to see something like this:

1. stash all changes that are not staged
2. do something that modifies files that have staged hunks (e.g. prettier, eslint)
3. stage any new modification of those files
4. pop all stashed changes

if 4. fails due to confllicts, abort commit and revert everything back to what it was before 1. and tell the user to fix things manually.

[naartjie]

I could fail the whole hook in that case and say that fixes can't be applied to a partially staged file and leave it up to you.
Does anyone think that would be acceptable?

@okonet I think it's acceptable, and actually desirable (to fail the whole hook).

DX: You've gone to all the trouble to git add --patch, when you commit an unexpected thing happens, any files with hunks staged are added in their entirety. I think a failure is more predictable than adding the whole file silently (well it appears in the diff, but it's easy to miss).

[alexilyaev]

Would really love to see this closed.

[okonet]

Who wouldn’t :) Any help us appreciated. See #75

[amit1911]

pre-commit seems to handle the partially staged files pretty well. What exactly are they doing differently?

[julienw]

They say:

Running hooks on unstaged changes can lead to both false-positives and false-negatives during
committing. pre-commit only runs on the staged contents of files by temporarily saving the contents of
your files at commit time and stashing the unstaged changes while running hooks.

[okonet]

@amit1911 they only run on the staged hunks which I also have done in #75 but the problem I'm facing is with the case when we modify those hunks during the command (i.e. prettier) and we need to "merge" back. Please take a look at the branch and tests to get a better sense of what I'm trying to do.

[karlhorky]

Does the approach taken by precise commits (formatting character ranges instead of full files) provide anything of interest?

[okonet]

@karlhorky not really. It might help with Prettier but afaik no other tool support this and lint-staged is being used with eslint and stylelint (both support --fix). Also there is #366 which is exactly about that.

[julien-f]

My current approach: https://github.com/vatesfr/xen-orchestra/blob/master/scripts/lint-staged

1. compute the list of files with unstaged changes
2. format all files
3. save in memory the files with unstaged changes and restore them
4. reformat them
5. run the tests and add the files if no errors
6. restore the files with unstaged changed to their saved state

[ntwb]

Currently, precise commits is the only pre-commit hook offering this functionality:

_"This is currently the only tool that will format only staged lines rather than the entire file"_

Thus, it is _of interest_ to me, for the projects where I don't use Prettier

[joeljeske]

I do not believe there is any reliable way to stash-modify-unstash changes to a partially staged without sometimes causing conflicts. I also do not think that it would occur very often in a typical flow. For that reason, I use a simple script to throw an error if a partially staged file is modified during a step.

https://gist.github.com/joeljeske/cf0b7e458e9f2a8d48fd6722b4141a73

If it detects an error, you could bypass it by simply running it again, as the second time there should not be any changes.

[sompylasar]

@joeljeske Thanks for sharing! Maybe this could be incorporated into lint-staged to reuse the code and avoid extra script setups?

[alexilyaev]

+1 for lint-staged doing the job for 95% of the cases and just throwing on a potential conflict.

[julien-f]

@joeljeske What's wrong with my approach?
We've been using it for some time and did not find any problems so far.

[joeljeske]

I don’t fully understand your suggestion. Could explain a bit more fully what you are doing?

[julien-f]

1. format the files that have changed (unstaged parts included)
2. stash those unstaged parts
3. re-format the files (unstaged parts no longer included)
4. run the tests
5. if tests succeeds, add the formatting changes to the git index and commit
6. unstash changes saved in 2

Results:

• the files will be fully formatted (staged and unstaged parts)
• only staged parts will be commited

[dannobytes]

wrt to eslint, I believe this is the actual command sequence that @julien-f is referring to:

  "lint-staged": {
    "**/*.js": [
      "eslint --fix",
      "git stash push -k -m 'tmp stash staged'",
      "eslint --fix",
      "git add",
      "git stash pop"
    ]
  }

However the last pop command runs into an error b/c all global paths are being passed to each command, thus:

/Users/home/somefile.js is not a valid reference
error An unexpected error occurred: "Command failed.

Seems like all we need here is a way to ignore path arguments being passed by lint-staged to a specific command, in this case, the last command in my list which is git stash pop.

I've also tried popping the stash in the precommit command after lint-staged runs instead like:

    "precommit": "lint-staged && git stash pop"
}
  "lint-staged": {
    "**/*.js": [
      "eslint --fix",
      "git stash push -k -m 'tmp stash staged'",
      "eslint --fix",
      "git add"
    ]}

This generally works except for the case where precommit runs when there are 0 files staged in the index, which then results in unexpected behavior where git stash pop attempts to pop a stash entry that was never created.

Personally, running eslint twice is unavoidable as discussed in prior comments. And to me, it resolves this issue to maintain unstaged hunks in a file.

@okonet All we need is a way to ignore the paths you're passing in to the last git stash pop command within the lint-staged config. Is there a way to do this in Unix that I'm not aware of? I've tried git stash pop; echo or something like that, but its not actually working as intended.

[dannobytes]

@okonet https://github.com/okonet/lint-staged/blob/master/src/runScript.js#L39 maybe here, you can add support for a ignore-paths flag that can disable the appending of pathChunks, i.e. each absolute file path to the command? By default, file paths are appended, but the consumer can selectively disable the paths if they want to run a command in lint-staged without them.

For example:

  "lint-staged": {
    "**/*.js": [
      "eslint --fix",
      "git add",
      "--ignore-paths mycommand_that_runs_with_no_path_args"
    ]}

[julien-f]

wrt to eslint, I believe this is the actual command sequence that @julien-f is referring to:

@dannobytes Nope, I'm talking about this script, I found it easier to implement it directly instead of relying on a sequence of commands.

[dannobytes]

@julien-f I didn't mean it literally, but more conceptually and within the
workflow that lint-staged was designed to do, which is sequential
commands run against staged files. Your script is fine, but it's IMHO not a
proposed solution to the existing workflow that most of us are using.

I like the sequential command approach as it's quite adaptable. If we just
added one small feature to allow running commands in the sequence without
file path args, it would be a minimal, opt-in, non-breaking change that
will add a bit more flexibility to commands being run during precommits.
And this would give us the workaround to support partially staged files
with unstaged hunks left in tact.

On Sun, Apr 15, 2018, 13:25 Julien Fontanet [e-mail]
wrote:

wrt to eslint, I believe this is the actual command sequence that
@julien-f https://github.com/julien-f is referring to:

@dannobytes https://github.com/dannobytes Nope, I'm talking about this
script
https://github.com/vatesfr/xen-orchestra/blob/master/scripts/lint-staged,
I found it easier to implement it directly instead of relying on a sequence
of commands.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/okonet/lint-staged/issues/62#issuecomment-381435278,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAt2r40-tbaqiTuUwMezFcABLOeloqc5ks5to6yhgaJpZM4KPwFp
.

[okonet]

@dannobytes this is being discussed in #138 FYI.

I'd like this workflow be incorporated into the library since I don't believe users should be concerned about the right sequence of commands. I'm working on a prototype in #75 but have still some tests failing regardless so if anyone would like to help, please ping me and we can pair on that.

@julien-f I'll try implementing your approach too.

[julienw]

Of interest to this discussion is git-autofixup. This uses an algorithm to try to match diff output to previous commits. The algorithm is quite simple (but requires some knowledge of Perl to read :) ) and relies on the output of git blame and git diff to match hunks with commits.

Of course the same algorithm couldn't be used here, but maybe something similar could work here: for partially staged files, after running the linters (prettier, eslint --fix, etc), we would add only the changes that are part of the hunks of the partially staged files. This can be done by analyzing the context lines of the new diff, and look whether they're part of the diff for these partially staged files.

That said, this isn't easy and can be difficult to tune.

[glasser]

So I've personally found it frustrating to be working on somebody else's repo that uses lint-staged and working with my usual git add -p workflow (or, well, Emacs magit) and suddenly have lint-staged overwrite what I'm trying to do.

I get why implementing lint-staged with git add -p is not straightforward, having lurked on this issue for a while. I wonder if there is a solution not of the form "magically make this complex thing work with git add -p" but of the form "just don't run the linter if there are non-added changes in a file"?

(Honestly what I'm really looking for is a global setting that lets me turn off lint-staged in the repos I work on that use it (all of which have other ways of invoking the linter that don't get in the way of my git habits), short of remembering to pass --no-verify to git commit every time.)

[EddieOne]

The most elegant solution i've found doesn't use this library 😢 Also, it requires node 8.x+

git diff --diff-filter=d --cached | grep '^[+-]' | grep -Ev '^(--- a/|\+\+\+ b/)' | npx eslint --stdin

Git diff, it shows mostly the line changes but not deleted ones. Pipe that into a regex to only get changed lines. Some regex I don't understand (combining stack overflows here). Then npx to solve file paths for dependencies. Then finally eslint. Combine with Husky, then take a nap 😴

[hkdobrev]

Some regex I don't understand

grep -Ev '^(--- a/|\+\+\+ b/)' is excluding (-v) with an extended regex (-E) the lines of the diff sections which mark the a and b sections of the diff with what was before --- and what is now +++.

[asmundg]

I ended up writing a small wrapper that executes a linter dry run on the staged files and only applies fixes and git adds if there is a diff.

This enables partial staging as long as your editor applies the correct formatting on save, which I find to be useful in any case.

[okonet]

Can you elaborate on what you did and can you share your code? Does anyone wants to take a stub on that issue? I have a good test case in place that should help.

[asmundg]

It's quite literally what I said above, i.e.

const diff = spawnSync("prettier", ["--list-different"].concat(files), { encoding: "utf8", env: process.env }).stdout.trim();

if (diff) {
    const diffFiles = diff.split(" ");
    console.log(`Files reformatted and staged: ${diffFiles.join(", ")}`);

    spawnSync("prettier", ["--write"].concat(diffFiles)), { env: process.env };
    spawnSync("git", ["add"].concat(diffFiles));
}

I couldn't think of any straight forward way of generalizing this. So it essentially removes the config from package.json into a linter-specific script.

[OliverJAsh]

For git add -p users who work on projects with lint-staged enabled in a precommit hook, like the one I work on, and who don't want to run lint-staged on precommit (because of the problems described in this issue), here is a way to configure your precommit hook to allow users to disable it: /cc @glasser

#!/bin/bash

# To disable, run `git config husky.disableHooks true`.

# Idea from https://github.com/typicode/husky/issues/258#issuecomment-386275270

hook() {
  lint-staged
}

if [[ `git config --get husky.disableHooks` == "true" ]]
then
  echo Husy hooks disabled. Skipping precommit hook.
else
  hook
fi

[glasser]

@OliverJAsh Cool. Does lint-staged/husky offer an easy way to customize the script, though?

I guess I can also just do something like have my "precommit" script check an env var.

[hallettj]

I implemented a little script to run a formatter / fixer on only staged changes. Then I saw that people on this project are working on the same problem, so I thought I would share the approach that is working quite well for me so far. See:

https://github.com/hallettj/git-format-staged/

I am still working on some details, such as handling cases where the working directory is not at the root of the git repository.

My strategy is to limit modifications to working tree files as much as possible. What I do is to run the formatter on git blobs, and create new git blobs with the output. Roughly like this:

NEW_HASH=$(git cat-file -p $OBJECT_HASH | $formatter | git hash-object -w --stdin)

Then swap the staged file content by switching blobs:

git update-index --cacheinfo "$MODE,$NEW_HASH,$PATH"

Finally to keep working tree files in sync with staged / committed changes my script optionally merges changes from formatting back to the working tree. It computes a patch by comparing the original object hash of the staged file with the hash after formatting, and applies that patch to the working tree:

git diff $OBJECT_HASH $NEW_HASH | git apply -

The patch actually needs to be massaged a bit to fix up working tree paths, but the above command gives the idea.

Patching working tree files will fail sometimes. I chose to make that a non-fatal warning. If the user proceeds to commit (for example if the script is invoked in a precommit hook) then the committed files will be formatted, and working tree files will be out of sync in cases where patching failed. But in many cases patching the working tree works transparently. Even if the working tree gets out of sync it is likely to get fixed up on subsequent changes and invocations of git-format-staged.

[okonet]

Thanks for sharing this! I’m using similar approach in the branch with the only difference that I “stash” local changes that aren’t in index and still work on files. The problem I’m facing is exactly you’ve described: sometimes syncing formatted changes with the working copy fails. The rest is working quite well in my branch.

[hallettj]

@okonet Sorry about the long delay replying. The difference in strategies results in different outcomes when there are conflicts between unstaged changes and formatting changes.

When you stash changes, format, and reapply stashed changes a conflict means that unstaged changes are not accessible until the conflict has been resolved.

git-format-staged formats the staged version of the file, and then attempts to merge formatting changes with unstaged changes. If the merge fails unstaged changes are kept intact, and the working tree file is left unformatted. The user does not have to deal with any conflicts. Working tree files are never modified except when they can be formatted cleanly. Staged/committed files are always formatted properly because the step of formatting the staged version of the file is independent from merging formatting into the working tree.

However to make its strategy work git-format-staged requires that the formatter or linter operates in a pipe, reading content from stdin and producing formatted output in stdout. That might clash with the design principles of lint-staged.

[okonet]

Staged/committed files are always formatted properly because the step of formatting the staged version of the file is independent from merging formatting into the working tree.

I was ably to achieve this in me PR #75 but I wasn't satisfied with that since it felt very frustrating to have non-formatted files in the working copy after I've committed formatted changes.

[viztor]

Also ran into this issue. Is there a way to manipulate git's internal file object to achieve the same idea?
Amazed by how long this thread has been there.

[edvardchen]

Inspired by git stash -k mentioned by many comments, here is my FINAL solution(more complex than initial version):

// package.json
......
"precommit": "/path/to/precommit.sh",
......
"lint-staged": {
  "*.js": ["eslint --fix ", "git add"]
},

# precommit.sh
message="precommit_on_$(git log -1 --format=%H)"
git stash -k -m$message

# because git stash always success
# we need to check if we have stashed successfully
lastStash=$(git stash list -1)
if [[ $lastStash == *$message* ]]; then
    # If stashed, git stash pop when exit
    trap 'git stash pop' EXIT
fi

# Here's what we intend to run
lint-staged

It should work on almost cases, including the cases following:

1. 0 files staged in the index
2. all files are staged
3. there are no local changes
4. configured lint-staged commands failed

[trusktr]

🤔 hmmm, @NoChapter's solution is nice. @okonet Would be great to make this an option for lint-staged!

[okonet]

Just a follow up here but I’m finishing my work on #75. I’ve published a beta version under the beta channel so if you have time please install and help me test it (I’m using it for past few days myself on clients project and everything seems fine).

npm install --save-dev lint-staged@beta

[Mouvedia]

Is npmD a typo? Are you using https://github.com/dominictarr/npmd?

[okonet]

It’s an alias I’m using. npm install --save-dev

[SimenB]

https://github.com/robbyrussell/oh-my-zsh/blob/f73c29a8203f8df539a72d28763bc5f521b775c0/plugins/npm/npm.plugin.zsh#L24-L26 :)

[trusktr]

@okonet You have

  const partiallyStaged = files.filter(file => file.startsWith('MM'))

but maybe it is better to have

  const partiallyStaged = files.filter(file => file.startsWith('M'))

because for example here's what I see on mine:

❯ git status --porcelain
 M package-lock.json
 M package.json
M  precommit.sh

There I've staged precommit, but it doesn't have MM.

Note also that staged changes to always start with M! For example, a rename starts with R.

So far, I think that unstaged changes always start with or ?, so the following is what I'm doing in my shell script to detect _unstaged_ changes, but I don't know if it is good enough:

# See https://github.com/okonet/lint-staged/issues/62

# check for unstaged changes (is there a better way?)
git status --porcelain | grep '^ \|^?'
result=$?
hasUnstagedChanges=false; if $( exit $result ); then hasUnstagedChanges=true; fi

if $hasUnstagedChanges; then
  echo "stashing unstaged changes"

  message="precommit_on_$(git log -1 --format=%H)"
  git stash -k -m$message

  # because git stash always success
  # we need to check if we have stashed successfully
  lastStash=$(git stash list -1)
  if [[ $lastStash == *$message* ]]; then
    # If stashed, git stash pop when exit
    trap 'git stash pop' EXIT
  fi
else
  echo "no unstaged changes, skipping stash"
fi

# Here's what we intend to run
lint-staged

It seems to work sometimes, but I get lots of conflicts when I pop the stash. I'm not sure why yet.

[trusktr]

@okonet also I think it should avoid stashing if there are no unstaged changes (if not already), otherwise empty stashes could be confusing.

EDIT, Ah, I missed it. Nice:

 ↓ Stashing changes... [skipped]
   → No unstaged files found...

[trusktr]

Alright, I tried it. It looks like maybe the MM issue caused it not to detect my unstaged changes:

Signafy+mapper-public-website git:use-jss ❯ git status
On branch use-jss
Your branch and 'origin/use-jss' have diverged,
and have 3 and 1 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

    modified:   src/components/Footer.tsx
    modified:   src/components/Header.tsx
    modified:   src/components/HeaderMobileNavModal.tsx
    modified:   src/components/RootLayout.tsx

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   package.json
    deleted:    src/App.vue
    deleted:    src/names.js
    modified:   src/pages/_document.tsx
    modified:   src/styles/theme.ts

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    src/App.vuee src/namess.js


❯ git commit -m "test"
husky > pre-commit (node v10.9.0)
 ↓ Stashing changes... [skipped]
   → No unstaged files found...
 ✔ Running linters...
[use-jss 696d898] test
 4 files changed, 309 insertions(+), 66 deletions(-)

[trusktr]

I tried modifying the code to

  const partiallyStaged = files.filter(file => !file.startsWith(' ') && !file.startsWith('?'))

but that was even worse, it ended up undoing file deletions that I had, and it still said I had no unstaged changes. For example:

❯ git status
On branch use-jss
Your branch and 'origin/use-jss' have diverged,
and have 3 and 1 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

    modified:   src/components/Footer.tsx
    modified:   src/components/Header.tsx
    modified:   src/components/HeaderMobileNavModal.tsx
    modified:   src/components/RootLayout.tsx

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   package.json
    deleted:    src/App.vue
    deleted:    src/names.js
    modified:   src/pages/_document.tsx
    modified:   src/styles/theme.ts

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    src/App.vuee src/namess.js


❯ git commit -m 'test'
husky > pre-commit (node v10.9.0)
 ↓ Stashing changes... [skipped]
   → No unstaged files found...
 ✔ Running linters...
 ✔ Updating index...
 ✔ Restoring local changes...
[use-jss 895ce96] test
 4 files changed, 309 insertions(+), 66 deletions(-)


❯ git status
On branch use-jss
Your branch and 'origin/use-jss' have diverged,
and have 4 and 1 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   package.json
    modified:   src/pages/_document.tsx
    modified:   src/styles/theme.ts

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    src/App.vuee src/namess.js

no changes added to commit (use "git add" and/or "git commit -a")


❯ git reset HEAD~
Unstaged changes after reset:
M   package-lock.json
M   package.json
M   precommit.sh
M   src/components/Footer.tsx
M   src/components/Header.tsx
M   src/components/HeaderMobileNavModal.tsx
M   src/components/RootLayout.tsx
M   src/pages/_document.tsx
M   src/styles/theme.ts

❯ git status
On branch use-jss
Your branch and 'origin/use-jss' have diverged,
and have 2 and 1 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   package-lock.json
    modified:   package.json
    modified:   precommit.sh
    modified:   src/components/Footer.tsx
    modified:   src/components/Header.tsx
    modified:   src/components/HeaderMobileNavModal.tsx
    modified:   src/components/RootLayout.tsx
    modified:   src/pages/_document.tsx
    modified:   src/styles/theme.ts

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    src/App.vuee src/namess.js

no changes added to commit (use "git add" and/or "git commit -a")

In the output, the last three commans (status, reset, and status) show the that the unstaged deleted changes are gone. It is as if I ran git reset on the deleted files, but in fact I didn't.

[okonet]

Not sure what you trying to achieve but you didn’t try to commit partially staged files so it worked as expected. BTW Please add comments to the PR instead so it’s not lost here.

[trusktr]

Looks like with MM or checking not and ?, it still works but with the same problem, but it always outputs No unstaged files found... every time regardless.

To test this fully, we should try combinations of

• have unstaged deleted files
• unstaged renamed files (has both deleted and untracked)
• untracked files (are not a rename)
• modified files

[trusktr]

Alright, I'll continue there.

[hallettj]

I think that this is a cleaner command for getting a list of partially staged files:

git diff-index --cached --diff-filter=AM --no-renames HEAD

--cached means show staged files
--diff-filter=AM shows only added (previously uncommitted) or modified files, which spares you from dealing with tricky cases like unmerged files
--no-renames disables rename detection; in combination with the diff-filter this means that if a file has been renamed it will only appear in the list with its new name

Edit: I don't mean to be overly pushy with my own lint-runner implementation, but feel free to reference git-format-staged while working on this issue.

[DAreRodz]

Hi, I just tried the beta version with staged hunks and it's working fine. Great work @okonet 😁

[okonet]

Yes that’s a known issue already

[rich4756]

Any update on this? ETA for next non-beta release with these changes included? Thank you very much!

[rich4756]

@hallettj The command git diff-index --cached --diff-filter=AM --no-renames HEAD is returning for me all staged files, not just partially staged files.

[julienw]

:tada:

[lcottereau]

thanks ! well done !

[alexilyaev]

This works really well, really appreciate the hard work!
Tested partial commits (hunks) and also when a linter fails everything is restored properly and not lost.

I'm curious if the note in the README about "Automatically fix code" is still relevant:

Automatically fix code style with --fix and add to commit

{
  "*.js": ["eslint --fix", "git add"]
}

This will run eslint --fix and automatically add changes to the commit. Please note, that it doesn’t work well with committing hunks (git add -p).

[okonet]

@alexilyaev good catch! Mind PRing the fix?

[alexilyaev]

@okonet https://github.com/okonet/lint-staged/pull/553

[asottile]

just a heads up since it came up in pre-commit -- I suspect you'll have issues using git stash with git add --intent-to-add files: https://marc.info/?l=git&m=154654843113132&w=2 (pre-commit doesn't use git stash)

not sure if it breaks lint-staged, I didn't try

[Mouvedia]

@asottile thanks for the heads-up. @okonet could you try it out?

[okonet]

We are also not using git stash. I can’t try since I lack time. If someone can create a test case that would be great.