Pass color option to git diff (on failure) #1051
Conversation
|
@asottile Build is failing on Windows, getting error : Can you help ? |
asottile
left a comment
There was a problem hiding this comment.
feel free to ignore the windows failures, azure pipelines added their own rust in a different place so it isn't picking it up -- I'll put in a fix for that
|
alright I've fixed the |
pre_commit/commands/run.py
Outdated
| '--color={}'.format(args.color), | ||
| )) | ||
| else: | ||
| subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff')) |
There was a problem hiding this comment.
I think basically what we're going to have to end up with is:
cmd = ('git', '--no-pager', 'diff', '--no-ext-diff')
if args.color:
cmd += ('--color=always',)
subprocess.call(cmd)Since at this point args.color will always be a boolean
tests/commands/run_test.py
Outdated
| ( | ||
| { | ||
| 'show_diff_on_failure': True, | ||
| 'color': 'auto', |
There was a problem hiding this comment.
so as above, this isn't a valid option here unfortunately
There was a problem hiding this comment.
But the tests pass :)
There was a problem hiding this comment.
yeahhhh the tests pass because we ignore the return code of git diff -- it actually ends up printing something like this:
$ git diff --color=True
error: option `color' expects "always", "auto", or "never"|
ah shoot sorry, I wasn't fast enough -- you were right /o\ |
|
@asottile Now the build is failing on
I don't have "write" access, so someone with write access will have to "ignore the error" and accept/merge the commit. |
|
I'm streaming right now if you want to chat about this PR -- https://twitch.tv/anthonywritescode :) |
|
the CI flakes are due to this I believe: coveragepy/coveragepy#807 hopefully I'll be able to fix them at some point :S |
|
@asottile Hopefully this is the last iteration. Please review and accept/merge the PR. |
|
Thanks! |
Fixes #1007