{"thread":{"id":"46845","subject":"Updated to v2.14.2 on macOS; git add --patch broken","startedAt":"2017-09-27T13:23:32Z","lastAt":"2017-10-20T13:31:23Z","messageCount":42,"participants":["Toni Uebernickel","Jeff King","Jonathan Nieder","Junio C Hamano","Tsvi Mostovicz","Jan Prachař"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"329036","messageId":"86D0A377-8AFD-460D-A90E-6327C6934DFC@gmail.com","threadId":"46845","inReplyTo":null,"subject":"Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Toni Uebernickel","fromEmail":"tuebernickel@gmail.com","sentAt":"2017-09-27T13:23:21Z","receivedAt":"2017-09-27T13:23:32Z","isPatch":false,"sender":{"key":"tuebernickel@gmail.com","avatar":"https://gravatar.com/avatar/5b63441a1dd6a38264525b55bc81181fa69af022a9dafff46610d8f7fd407785?d=mp&s=160"},"body":"Hi there,\n\nI updated to git version v2.14.2 on macOS using homebrew.\n\nSince then `git add --patch` and `git stash save --patch` are not working anymore. It's just printing the complete diff without ever stopping to ask for actions. This results in an unusable state, as the whole command option is rendered useless.\n\nFor illustration see: https://asciinema.org/a/fw6cy9Ds6CeNOt4RYknLYgOD3\n\nKind Regards,\nToni\n\n--\nToni Uebernickel\n\ntuebernickel@gmail.com - https://keybase.io/havvg\nhttps://github.com/havvg - https://www.xing.com/profile/Toni_Uebernickel\n\n"},{"id":"329043","messageId":"20170927170716.slqxje2tkh5ws5ak@sigill.intra.peff.net","threadId":"46845","inReplyTo":"86D0A377-8AFD-460D-A90E-6327C6934DFC@gmail.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-27T17:07:16Z","receivedAt":"2017-09-27T17:07:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 27, 2017 at 03:23:21PM +0200, Toni Uebernickel wrote:\n\n> Hi there,\n> \n> I updated to git version v2.14.2 on macOS using homebrew.\n> \n> Since then `git add --patch` and `git stash save --patch` are not\n> working anymore. It's just printing the complete diff without ever\n> stopping to ask for actions. This results in an unusable state, as the\n> whole command option is rendered useless.\n\nWhat was the previous version that was working? It is possible to bisect\nto find the culprit commit?\n\nThere weren't any updates to git-add--interactive.perl (the program that\nunderlies the `--patch` implementation in both cases) in v2.14.2, and\nvery few even in v2.14.0. Of course the problem may be in one of the\nsub-programs it calls.\n\n-Peff\n"},{"id":"329046","messageId":"38D148EC-B77D-426E-A7E0-128509590407@gmail.com","threadId":"46845","inReplyTo":"20170927170716.slqxje2tkh5ws5ak@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Toni Uebernickel","fromEmail":"tuebernickel@gmail.com","sentAt":"2017-09-27T17:28:49Z","receivedAt":"2017-09-27T17:28:57Z","isPatch":false,"sender":{"key":"tuebernickel@gmail.com","avatar":"https://gravatar.com/avatar/5b63441a1dd6a38264525b55bc81181fa69af022a9dafff46610d8f7fd407785?d=mp&s=160"},"body":"The previous version was v2.13.2.\nI switched back to this version, and it works perfectly fine; without any changes to my system.\n\n--\nToni Uebernickel\n\ntuebernickel@gmail.com - https://keybase.io/havvg\nhttps://github.com/havvg - https://www.xing.com/profile/Toni_Uebernickel\n\n> On 27. Sep 2017, at 19:07, Jeff King <peff@peff.net> wrote:\n> \n> On Wed, Sep 27, 2017 at 03:23:21PM +0200, Toni Uebernickel wrote:\n> \n>> Hi there,\n>> \n>> I updated to git version v2.14.2 on macOS using homebrew.\n>> \n>> Since then `git add --patch` and `git stash save --patch` are not\n>> working anymore. It's just printing the complete diff without ever\n>> stopping to ask for actions. This results in an unusable state, as the\n>> whole command option is rendered useless.\n> \n> What was the previous version that was working? It is possible to bisect\n> to find the culprit commit?\n> \n> There weren't any updates to git-add--interactive.perl (the program that\n> underlies the `--patch` implementation in both cases) in v2.14.2, and\n> very few even in v2.14.0. Of course the problem may be in one of the\n> sub-programs it calls.\n> \n> -Peff\n\n"},{"id":"329049","messageId":"20170927180127.khwqwmv74tsji5im@sigill.intra.peff.net","threadId":"46845","inReplyTo":"38D148EC-B77D-426E-A7E0-128509590407@gmail.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-27T18:01:27Z","receivedAt":"2017-09-27T18:01:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 27, 2017 at 07:28:49PM +0200, Toni Uebernickel wrote:\n\n> The previous version was v2.13.2.\n> I switched back to this version, and it works perfectly fine; without any changes to my system.\n\nThanks for confirming.\n\nThere aren't a lot of changes to the script between v2.13.2 and v2.14.2.\nThe most plausible culprit is d5addcf522 (add--interactive: handle EOF\nin prompt_yesno, 2017-06-21), but I'm scratching my head over how that\ncould cause what you're seeing.\n\nAre you able to build Git from source and bisect the problem? It would\nhelp to know which commit introduced the problem.\n\n-Peff\n"},{"id":"329055","messageId":"20170927195159.GE19555@aiede.mtv.corp.google.com","threadId":"46845","inReplyTo":"20170927180127.khwqwmv74tsji5im@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-27T19:51:59Z","receivedAt":"2017-09-27T19:52:08Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n> On Wed, Sep 27, 2017 at 07:28:49PM +0200, Toni Uebernickel wrote:\n\n>> The previous version was v2.13.2.\n>> I switched back to this version, and it works perfectly fine; without any changes to my system.\n>\n> Thanks for confirming.\n>\n> There aren't a lot of changes to the script between v2.13.2 and v2.14.2.\n> The most plausible culprit is d5addcf522 (add--interactive: handle EOF\n> in prompt_yesno, 2017-06-21), but I'm scratching my head over how that\n> could cause what you're seeing.\n>\n> Are you able to build Git from source and bisect the problem? It would\n> help to know which commit introduced the problem.\n\nHow about this change?\n\n commit 136c8c8b8fa39f1315713248473dececf20f8fe7\n Author: Jeff King <peff@peff.net>\n Date:   Thu Jul 13 11:07:03 2017 -0400\n\n     color: check color.ui in git_default_config()\n\nToni, what is the output of \"git config -l\"?\n\nThanks,\nJonathan\n"},{"id":"329056","messageId":"20170927195310.GF19555@aiede.mtv.corp.google.com","threadId":"46845","inReplyTo":"20170927195159.GE19555@aiede.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-27T19:53:10Z","receivedAt":"2017-09-27T19:53:19Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Jeff King wrote:\n\n>> There aren't a lot of changes to the script between v2.13.2 and v2.14.2.\n>> The most plausible culprit is d5addcf522 (add--interactive: handle EOF\n>> in prompt_yesno, 2017-06-21), but I'm scratching my head over how that\n>> could cause what you're seeing.\n>>\n>> Are you able to build Git from source and bisect the problem? It would\n>> help to know which commit introduced the problem.\n>\n> How about this change?\n>\n>  commit 136c8c8b8fa39f1315713248473dececf20f8fe7\n>  Author: Jeff King <peff@peff.net>\n>  Date:   Thu Jul 13 11:07:03 2017 -0400\n>\n>      color: check color.ui in git_default_config()\n\nUh, I think I was thinking of another thread when I wrote this.  Sorry\nfor the nonsense.\n\n> Toni, what is the output of \"git config -l\"?\n\nI'm still curious about this.\n\nThanks,\nJonathan\n"},{"id":"329095","messageId":"21387D23-F581-4179-969E-823FEF82C405@gmail.com","threadId":"46845","inReplyTo":"20170927195310.GF19555@aiede.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Toni Uebernickel","fromEmail":"tuebernickel@gmail.com","sentAt":"2017-09-28T05:03:55Z","receivedAt":"2017-09-28T05:04:07Z","isPatch":false,"sender":{"key":"tuebernickel@gmail.com","avatar":"https://gravatar.com/avatar/5b63441a1dd6a38264525b55bc81181fa69af022a9dafff46610d8f7fd407785?d=mp&s=160"},"body":"Hi Jonathan,\n\nmy configuration reads as follows, I only removed private tokens content.\nI will try to get more details on which version exactly breaks the command.\n\nKind Regards,\nToni\n\ncore.excludesfile=/usr/local/etc/gitignore\ncore.whitespace=trailing-space,space-before-tab,-indent-with-non-tab,tab-in-indent,tabwidth=2\ncore.autocrlf=input\ncore.ignorecase=true\ngpg.program=gpg2\ncredential.helper=osxkeychain\ndiff.patience=true\ndiff.algorithm=patience\ndiff.compactionheuristic=true\nnotes.displayref=refs/notes/*\nadvice.detachedhead=false\npull.rebase=preserve\npush.default=upstream\nmerge.ff=false\nmerge.summary=true\nmerge.log=false\nrerere.enabled=true\nrebase.autosquash=true\nrebase.autostash=true\nstash.showpatch=true\nbranch.autosetuprebase=always\ncolor.ui=always\ncolor.branch.current=yellow reverse\ncolor.branch.local=yellow\ncolor.branch.remote=green\ncolor.diff.meta=yellow\ncolor.diff.func=cyan bold\ncolor.diff.frag=magenta\ncolor.diff.old=red\ncolor.diff.new=green\ncolor.diff.whitespace=red reverse\ncolor.status.added=green\ncolor.status.changed=yellow\ncolor.status.untracked=cyan\ncolor.status.nobranch=red\npretty.changelog=format:* %h %s\npretty.public-changelog=format:* %s\npretty.pretty-history=format:%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset\ngitflow.multi-hotfix=true\ngitflow.prefix.feature=feature/\ngitflow.prefix.release=release/\ngitflow.prefix.bugfix=bugfix/\ngitflow.prefix.hotfix=hotfix/\ngitflow.prefix.support=support/\ngitflow.prefix.versiontag=v\ngitflow.feature.finish.rebase=true\ngitflow.feature.finish.no-ff=true\ngitflow.feature.finish.keepremote=true\ngitflow.feature.finish.keeplocal=false\ngitflow.release.finish.keepremote=true\ngitflow.release.finish.keeplocal=false\ngitflow.hotfix.finish.keepremote=true\ngitflow.hotfix.finish.keeplocal=false\nalias.st=status\nalias.co=checkout\nalias.di=diff\nalias.ci=commit\nalias.ci-rm=!git commit && git notes --ref=redmine add\nalias.ci-gh=!git commit && git notes --ref=github.issues add\nalias.ci-n=!git commit && git notes add\nalias.amend=commit --amend\nalias.update=add -u\nalias.up-head=!f() { git diff-tree --no-commit-id --name-only --diff-filter=ACMR -r HEAD; }; git add -p `f`\nalias.staged=diff --cached\nalias.ls-staged=diff-index --cached --name-only HEAD\nalias.di-staged=!git diff-index --cached --name-only --diff-filter=ACMR HEAD | xargs git di\nalias.co-staged=!git diff-index --cached --name-only --diff-filter=ACMR HEAD | xargs git co\nalias.up-staged=!f() { git ls-staged; }; git add -p `f`\nalias.coding-standards=!php-cs-fixer fix\nalias.cs=!f() { git diff-tree --no-commit-id --name-only --diff-filter=ACMR -r \"$1\" | xargs -n1 git coding-standards; }; f\nalias.cs-staged=!git diff-index --cached --name-only --diff-filter=ACMR HEAD | xargs -n1 git coding-standards\nalias.cs-head=!git diff-tree --no-commit-id --name-only --diff-filter=ACMR -r HEAD | xargs -n1 git coding-standards\nalias.cs-tree=!f() { git ls-tree -r --name-only HEAD \"$1\" | xargs -n1 git coding-standards; }; f\nalias.ph=log --graph --pretty=pretty-history --abbrev-commit --date=relative\nalias.cl=log --pretty=changelog  --no-merges --cherry --abbrev-commit --date-order\nalias.sl=log --oneline --reverse --no-merges --stat\nalias.rv=remote -v\nalias.fa=fetch --all -v\nalias.ff=pull --ff-only\nalias.wu=branch -a --no-merged\nalias.latest-version=!git tag --sort=v:refname | tail -n1\nalias.pt=push --tags origin master:master develop:develop\ngit-up.fetch.prune=true\ngit-up.fetch.all=true\nfilter.lfs.clean=git-lfs clean %f\nfilter.lfs.smudge=git-lfs smudge %f\nfilter.lfs.required=true\nuser.name=Toni Uebernickel\nuser.email=tuebernickel@gmail.com\nuser.signingkey=F6035C7A\ncore.editor=mate -w -l 1\ncommit.gpgsign=true\ngithub.user=havvg\ngithub.token=PRIVATE_CREDENTIALS_REMOVED\ntravis.user=tuebernickel@gmail.com\ntravis.token=PRIVATE_CREDENTIALS_REMOVED\nalias.review=!f() { git ph --author=\"$1\" ; }; f\nalias.rf=!f() { git flow feature track \"$1\" && git co feature/\"$1\" && git ff && git flow finish; }; f\nalias.upstream=!git merge --ff-only upstream/`git branch --list --no-color | grep -e '^\\*' | cut -d\" \" -f2\ncore.repositoryformatversion=0\ncore.filemode=true\ncore.bare=false\ncore.logallrefupdates=true\ncore.ignorecase=true\ncore.precomposeunicode=true\nremote.origin.url=git@gitlab.tarifhaus.ag:tarifhaus/thengine.git\nremote.origin.fetch=+refs/heads/*:refs/remotes/origin/*\nbranch.develop.remote=origin\nbranch.develop.merge=refs/heads/develop\nbranch.develop.rebase=true\ncommit.gpgsign=false\nbranch.master.remote=origin\nbranch.master.merge=refs/heads/master\nbranch.master.rebase=true\ngitflow.branch.master=master\ngitflow.branch.develop=develop\ngitflow.path.hooks=/Users/toni.uebernickel/Development/Tarifhaus/repositories/thengine/.git/hooks\nuser.email=toni.uebernickel@tarifhaus.ag\ngitflow.branch.feature/ITTPL-36-coupon-token-email-limit.base=develop\ngitflow.branch.feature/ITTPL-49-command-locking.base=develop\ngitflow.branch.release/2.5.0.base=develop\ngitflow.branch.feature/ITTHE-243-landingpage-content-tags.base=develop\ngitflow.branch.release/2.6.0.base=develop\ngitflow.branch.feature/ITTHE-246-json-web-token.base=develop\ngitflow.branch.release/2.7.0.base=develop\ngitflow.branch.feature/ITTHE-249-post-request-daten.base=develop\ngitflow.branch.feature/ITTHE-248-users-api.base=develop\ngitflow.branch.release/2.8.0.base=develop\ngitflow.branch.feature/ITTPL-92-url-shortener.base=develop\ngitflow.branch.release/2.9.0.base=develop\ngitflow.branch.feature/ITTHE-255-basic-orders-api.base=develop\ngitflow.branch.feature/ITTHE-260-migrate-databases-helper.base=develop\ngitflow.branch.feature/ITTHE-257-signup.base=develop\ngitflow.branch.feature/ITTHE-256-api-profile.base=develop\ngitflow.branch.feature/ITTHE-267-convert-to-testdox.base=develop\ngitflow.branch.release/2.10.0.base=develop\ngitflow.branch.feature/ITTPL-137-besser-mobile-portal.base=develop\ngitflow.branch.feature/ITTHE-251-HTTP-500-api-problem.base=develop\ngitflow.branch.feature/ITTHE-272-message-queue.base=develop\ngitflow.branch.feature/ITTBO-7-einfache-faq-verwaltung.base=hotfix/2.9.2\ngitflow.branch.feature/Tarifporfolio-V3.base=master\ngitflow.branch.release/2.11.0.base=develop\ngitflow.branch.release/2.12.0.base=develop\nbranch.support/ITTHE-262.remote=origin\nbranch.support/ITTHE-262.merge=refs/heads/support/ITTHE-262\nbranch.support/ITTHE-262.rebase=true\ngitflow.branch.feature/ITTHE-277-change-password.base=develop\ngitflow.branch.feature/ITTHE-280-TEF-MyPay.base=develop\ngitflow.branch.feature/ITTHE-292-msisdn-on-contract-activation.base=release/2.14.0\ngitflow.branch.feature/ITTHE-280-mypay-vouchercode-transactions.base=release/2.14.0\nbranch.feature/ITTHE-280-mypay-vouchercode-transactions.remote=origin\nbranch.feature/ITTHE-280-mypay-vouchercode-transactions.merge=refs/heads/feature/ITTHE-280-mypay-vouchercode-transactions\nbranch.feature/ITTHE-280-mypay-vouchercode-transactions.rebase=true\ngitflow.branch.feature/ITTHE-293-translate-security-error-messages.base=develop\ngitflow.branch.feature/ITTHE-295-combined-contract-endpoint.base=release/2.14.0\ngitflow.branch.feature/ITTHE-294-simcard-activation.base=release/2.14.0\nbranch.support/json-rpc-form-type.remote=origin\nbranch.support/json-rpc-form-type.merge=refs/heads/support/json-rpc-form-type\nbranch.support/json-rpc-form-type.rebase=true\ngitflow.branch.feature/ITTHE-310-user-profile-for-existing-users.base=develop\ngitflow.branch.feature/ITTHE-312-refactor-obtaining-vouchers.base=develop\ngitflow.branch.feature/ITTHE-298-vouchers-for-user-subset.base=develop\ngitflow.branch.feature/ITTHE-313-send-signup-message.base=develop\ngitflow.branch.release/2.16.0.base=develop\ngitflow.branch.release/2.17.0.base=develop\ngitflow.branch.feature/ITTHE-315-explicit-persist-attribute-value.base=develop\ngitflow.branch.release/2.18.0.base=develop\ngitflow.branch.feature/ITTPL-350-bcc-club-email-to-lbe.base=develop\ngitflow.branch.feature/ITTHE-321-promotion-voucher-on-contract-basis.base=develop\ngitflow.branch.feature/ITTHE-325-lock-voucher-when-obtaining.base=develop\ngitflow.branch.release/2.19.0.base=develop\ngitflow.branch.hotfix/2.19.1.base=master\ngitflow.branch.feature/ITTHE-339-api-access-control.base=develop\ngitflow.branch.release/2.20.0.base=develop\ngitflow.branch.feature/ITTHE-334-promotion-for-all-customers.base=develop\ngitflow.branch.feature/ITTHE-331-behat-context-api-problem.base=develop\ngitflow.branch.feature/ITTHE-336-canonical-user-email-address.base=develop\ngitflow.branch.feature/ITTHE-138-address-validation-setup-wrapper.base=develop\nlfs.https://gitlab.tarifhaus.ag/tarifhaus/thengine.git/info/lfs.locksverify=false\ngitflow.branch.feature/ITTHE-351-behat-assert-response-content.base=develop\ngitflow.branch.release/2.21.0.base=develop\ngitflow.branch.feature/ITTHE-355-refactor-chat-api.base=develop\ngitflow.branch.feature/ITADM-48-feature-deployments.base=develop\ngitflow.branch.release/2.22.0.base=develop\ngitflow.branch.feature/ITTHE-360-media-service.base=develop\nbranch.feature/ITTHE-360-media-service.remote=origin\nbranch.feature/ITTHE-360-media-service.merge=refs/heads/feature/ITTHE-360-media-service\nbranch.feature/ITTHE-360-media-service.rebase=true\ngitflow.branch.feature/tarifhaus-club-support.base=develop\ngitflow.branch.feature/ITTHE-370-open-promotions-not-listed.base=develop\ngitflow.branch.feature/ITTHE-371-support-registration-vouchers.base=develop\ngitflow.branch.release/2.23.0.base=develop\ngitflow.branch.feature/ITTHE-375-support-promotion-participants.base=develop\ngitflow.branch.feature/ITTHE-288-aboalarm-bank-account-check.base=develop\ngitflow.branch.release/2.25.0.base=develop\nbranch.feature/ITTHE-384-products-and-attributes-new-portal-strategy.remote=origin\nbranch.feature/ITTHE-384-products-and-attributes-new-portal-strategy.merge=refs/heads/feature/ITTHE-384-products-and-attributes-new-portal-strategy\nbranch.feature/ITTHE-384-products-and-attributes-new-portal-strategy.rebase=true\ngitflow.branch.feature/ITTHE-388-json-rpc.base=develop\ngitflow.branch.feature/ITTHE-382-anonymise-order-notification.base=develop\nbranch.feature/ITTHE-388-json-rpc.remote=origin\nbranch.feature/ITTHE-388-json-rpc.merge=refs/heads/feature/ITTHE-388-json-rpc\nbranch.feature/ITTHE-388-json-rpc.rebase=true\ngitflow.branch.feature/ITTHE-372-vouchers-on-monthly-basis.base=develop\nbranch.feature/ITTHE-385-timeline.remote=origin\nbranch.feature/ITTHE-385-timeline.merge=refs/heads/feature/ITTHE-385-timeline\nbranch.feature/ITTHE-385-timeline.rebase=true\ngitflow.branch.release/2.26.0.base=develop\nbranch.release/2.26.0.remote=origin\nbranch.release/2.26.0.merge=refs/heads/release/2.26.0\nbranch.release/2.26.0.rebase=true\nbranch.feature/ITTHE-386-member-connections.remote=origin\nbranch.feature/ITTHE-386-member-connections.merge=refs/heads/feature/ITTHE-386-member-connections\nbranch.feature/ITTHE-386-member-connections.rebase=true\n\n--\nToni Uebernickel\n\ntuebernickel@gmail.com - https://keybase.io/havvg\nhttps://github.com/havvg - https://www.xing.com/profile/Toni_Uebernickel\n\n> On 27. Sep 2017, at 21:53, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> \n> Jonathan Nieder wrote:\n>> Jeff King wrote:\n> \n>>> There aren't a lot of changes to the script between v2.13.2 and v2.14.2.\n>>> The most plausible culprit is d5addcf522 (add--interactive: handle EOF\n>>> in prompt_yesno, 2017-06-21), but I'm scratching my head over how that\n>>> could cause what you're seeing.\n>>> \n>>> Are you able to build Git from source and bisect the problem? It would\n>>> help to know which commit introduced the problem.\n>> \n>> How about this change?\n>> \n>> commit 136c8c8b8fa39f1315713248473dececf20f8fe7\n>> Author: Jeff King <peff@peff.net>\n>> Date:   Thu Jul 13 11:07:03 2017 -0400\n>> \n>>     color: check color.ui in git_default_config()\n> \n> Uh, I think I was thinking of another thread when I wrote this.  Sorry\n> for the nonsense.\n> \n>> Toni, what is the output of \"git config -l\"?\n> \n> I'm still curious about this.\n> \n> Thanks,\n> Jonathan\n\n"},{"id":"329097","messageId":"20170928052043.vh5wyecibe3cujwp@sigill.intra.peff.net","threadId":"46845","inReplyTo":"21387D23-F581-4179-969E-823FEF82C405@gmail.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-28T05:20:44Z","receivedAt":"2017-09-28T05:21:03Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 28, 2017 at 07:03:55AM +0200, Toni Uebernickel wrote:\n\n> color.ui=always\n\nThis is the problem, and Jonathan's guess was correct that 136c8c8b8f\n(color: check color.ui in git_default_config(), 2017-07-13) is related.\n\nRe-reading that commit message, I'm inclined to say that the commit\nisn't wrong, and that setting color.ui to \"always\" has always been a bad\nidea. Of course, I also wrote that commit message, so I may be biased. :)\n\nWhat led you to setting color.ui to \"always\" in your config?  The more\nusual value is \"auto\" (which is also the default these days).\n\n-Peff\n"},{"id":"329098","messageId":"A96CA99A-B2A1-4CC1-ADEC-128211C88B57@gmail.com","threadId":"46845","inReplyTo":"20170928052043.vh5wyecibe3cujwp@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Toni Uebernickel","fromEmail":"tuebernickel@gmail.com","sentAt":"2017-09-28T05:31:45Z","receivedAt":"2017-09-28T05:31:54Z","isPatch":false,"sender":{"key":"tuebernickel@gmail.com","avatar":"https://gravatar.com/avatar/5b63441a1dd6a38264525b55bc81181fa69af022a9dafff46610d8f7fd407785?d=mp&s=160"},"body":"That's a very very old setting :)\nHonestly, I don't know why it's \"always\". I have set up this setting years ago and never thought about it again, as it worked out.\n\nI changed it to \"auto\" and the --patch options are working again on v2.14.2!\n\nThank you very much for your time & efforts.\nIf I can be of any further help on this matter, just let me know.\n\nKind Regards,\nToni\n\n--\nToni Uebernickel\n\ntuebernickel@gmail.com - https://keybase.io/havvg\nhttps://github.com/havvg - https://www.xing.com/profile/Toni_Uebernickel\n\n> On 28. Sep 2017, at 07:20, Jeff King <peff@peff.net> wrote:\n> \n> On Thu, Sep 28, 2017 at 07:03:55AM +0200, Toni Uebernickel wrote:\n> \n>> color.ui=always\n> \n> This is the problem, and Jonathan's guess was correct that 136c8c8b8f\n> (color: check color.ui in git_default_config(), 2017-07-13) is related.\n> \n> Re-reading that commit message, I'm inclined to say that the commit\n> isn't wrong, and that setting color.ui to \"always\" has always been a bad\n> idea. Of course, I also wrote that commit message, so I may be biased. :)\n> \n> What led you to setting color.ui to \"always\" in your config?  The more\n> usual value is \"auto\" (which is also the default these days).\n> \n> -Peff\n\n"},{"id":"329488","messageId":"20171002230017.GT19555@aiede.mtv.corp.google.com","threadId":"46845","inReplyTo":"86D0A377-8AFD-460D-A90E-6327C6934DFC@gmail.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-10-02T23:00:17Z","receivedAt":"2017-10-02T23:00:27Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nToni Uebernickel wrote:\n\n> I updated to git version v2.14.2 on macOS using homebrew.\n>\n> Since then `git add --patch` and `git stash save --patch` are not\n> working anymore. It's just printing the complete diff without ever\n> stopping to ask for actions. This results in an unusable state, as\n> the whole command option is rendered useless.\n\nWould a patch like the following help?\n\nI am worried that other scripts using diff-files would need the same\nkind of patch.  So it seems worthwhile to look for alternatives.\n\nAn alternative would be to partially roll back v2.14.2~61^2~4 (color:\ncheck color.ui in git_default_config, 2017-07-13) by making it not\ntake effect for plumbing commands (i.e., by adding a boolean to\n\"struct startup_info\" to indicate whether a command is low-level\nplumbing).  That would make the behavior of Git harder to explain so I\ndon't particularly like it.  Plus it defeats the point of the patch.\n\nYet another alternative would be to treat color.ui=always as a\ndeprecated synonym for color.ui=auto.  I think that's my preferred\nfix.\n\nWhat do you think?\n\nThanks again for reporting,\nJonathan\n\ndiff --git i/git-add--interactive.perl w/git-add--interactive.perl\nindex 28b325d754..4ea69538c7 100755\n--- i/git-add--interactive.perl\n+++ w/git-add--interactive.perl\n@@ -101,49 +101,49 @@ sub apply_patch_for_stash;\n \n my %patch_modes = (\n \t'stage' => {\n-\t\tDIFF => 'diff-files -p',\n+\t\tDIFF => 'diff-files --no-color -p',\n \t\tAPPLY => sub { apply_patch 'apply --cached', @_; },\n \t\tAPPLY_CHECK => 'apply --cached',\n \t\tFILTER => 'file-only',\n \t\tIS_REVERSE => 0,\n \t},\n \t'stash' => {\n-\t\tDIFF => 'diff-index -p HEAD',\n+\t\tDIFF => 'diff-index --no-color -p HEAD',\n \t\tAPPLY => sub { apply_patch 'apply --cached', @_; },\n \t\tAPPLY_CHECK => 'apply --cached',\n \t\tFILTER => undef,\n \t\tIS_REVERSE => 0,\n \t},\n \t'reset_head' => {\n-\t\tDIFF => 'diff-index -p --cached',\n+\t\tDIFF => 'diff-index --no-color -p --cached',\n \t\tAPPLY => sub { apply_patch 'apply -R --cached', @_; },\n \t\tAPPLY_CHECK => 'apply -R --cached',\n \t\tFILTER => 'index-only',\n \t\tIS_REVERSE => 1,\n \t},\n \t'reset_nothead' => {\n-\t\tDIFF => 'diff-index -R -p --cached',\n+\t\tDIFF => 'diff-index --no-color -R -p --cached',\n \t\tAPPLY => sub { apply_patch 'apply --cached', @_; },\n \t\tAPPLY_CHECK => 'apply --cached',\n \t\tFILTER => 'index-only',\n \t\tIS_REVERSE => 0,\n \t},\n \t'checkout_index' => {\n-\t\tDIFF => 'diff-files -p',\n+\t\tDIFF => 'diff-files --no-color -p',\n \t\tAPPLY => sub { apply_patch 'apply -R', @_; },\n \t\tAPPLY_CHECK => 'apply -R',\n \t\tFILTER => 'file-only',\n \t\tIS_REVERSE => 1,\n \t},\n \t'checkout_head' => {\n-\t\tDIFF => 'diff-index -p',\n+\t\tDIFF => 'diff-index --no-color -p',\n \t\tAPPLY => sub { apply_patch_for_checkout_commit '-R', @_ },\n \t\tAPPLY_CHECK => 'apply -R',\n \t\tFILTER => undef,\n \t\tIS_REVERSE => 1,\n \t},\n \t'checkout_nothead' => {\n-\t\tDIFF => 'diff-index -R -p',\n+\t\tDIFF => 'diff-index --no-color -R -p',\n \t\tAPPLY => sub { apply_patch_for_checkout_commit '', @_ },\n \t\tAPPLY_CHECK => 'apply',\n \t\tFILTER => undef,\n"},{"id":"329511","messageId":"xmqqpoa5kp0c.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171002230017.GT19555@aiede.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T01:18:27Z","receivedAt":"2017-10-03T01:18:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Yet another alternative would be to treat color.ui=always as a\n> deprecated synonym for color.ui=auto.  I think that's my preferred\n> fix.\n\nIt is mine, too.  And I do not think color.ui=absolutely-always for\nthose who want to keep the current behaviour is unneeded.\n\n"},{"id":"329515","messageId":"xmqqinfxklw7.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"xmqqpoa5kp0c.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T02:25:44Z","receivedAt":"2017-10-03T02:25:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>> Yet another alternative would be to treat color.ui=always as a\n>> deprecated synonym for color.ui=auto.  I think that's my preferred\n>> fix.\n>\n> It is mine, too.  And I do not think color.ui=absolutely-always for\n> those who want to keep the current behaviour is unneeded.\n\nHaving said that, I do not mind solving what 136c8c8b (\"color: check\ncolor.ui in git_default_config()\", 2017-07-13) tried to do in a\ndifferent way.  If 4c7f1819b that made even plumbing to default to\nauto was wrong (because plumbing did not pay attention to color.ui\nso people could not override that 'auto' default), we can partially\nrevert 4c7f1819 (\"make color.ui default to 'auto'\", 2013-06-10) to\nmake the default 'auto' not apply to plumbing.\n\nIn any case, I think the first step may be to revert 136c8c8b from\nboth 'master' and 2.14.x.  These alternative solutions can come on\ntop.\n\nThoughts?\n"},{"id":"329516","messageId":"xmqqefqkly87.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"xmqqinfxklw7.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T03:14:00Z","receivedAt":"2017-10-03T03:14:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> In any case, I think the first step may be to revert 136c8c8b from\n> both 'master' and 2.14.x.  These alternative solutions can come on\n> top.\n>\n> Thoughts?\n\nWith the attached patch, after reverting 136c8c8b (\"color: check\ncolor.ui in git_default_config()\", 2017-07-13) from the tip of\n'master', all tests seem to pass.  More importantly,\n\n    git -c color.ui=always diff-files -p >not-a-tty\n\nwill not get colors on its output file, because it does not pay\nattention to color.ui configuration.\n\nNote that I haven't _decided_ that reverting is the best way to move\nforward (yet).  I am just giving a datapoint for people to use when\nthey assess how painful each possible avenues proposed are.\n\n builtin/for-each-ref.c | 3 ++-\n builtin/tag.c          | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 5d7c921a77..238eb00e09 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -5,6 +5,7 @@\n #include \"object.h\"\n #include \"parse-options.h\"\n #include \"ref-filter.h\"\n+#include \"color.h\"\n \n static char const * const for_each_ref_usage[] = {\n \tN_(\"git for-each-ref [<options>] [<pattern>]\"),\n@@ -54,7 +55,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \n \tformat.format = \"%(objectname) %(objecttype)\\t%(refname)\";\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(git_color_default_config, NULL);\n \n \tparse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);\n \tif (maxcount < 0) {\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex c627794181..f8bc1393ed 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -158,7 +158,7 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static void write_tag_body(int fd, const struct object_id *oid)\n\n\n"},{"id":"329519","messageId":"20171003040737.2336-1-gitster@pobox.com","threadId":"46845","inReplyTo":"20171002230017.GT19555@aiede.mtv.corp.google.com","subject":"[PATCH 0/2] fixing \"add -p\" regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T04:07:35Z","receivedAt":"2017-10-03T04:07:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We were a bit too agressive in pushing color.ui configuration to\nplumbing commands.  A real fix must be found to override the default\n\"auto\" use of colors for any color-capable plumbing commands, but\nlet's leave that to a later round and concentrate on fixing the\nregression first.\n\nJunio C Hamano (2):\n  Revert \"color: check color.ui in git_default_config()\"\n  colors: git_default_config() does not read color.ui\n\n builtin/branch.c       | 2 +-\n builtin/clean.c        | 3 ++-\n builtin/for-each-ref.c | 3 ++-\n builtin/grep.c         | 2 +-\n builtin/show-branch.c  | 2 +-\n builtin/tag.c          | 2 +-\n color.c                | 8 ++++++++\n config.c               | 4 ----\n diff.c                 | 3 +++\n 9 files changed, 19 insertions(+), 10 deletions(-)\n\n-- \n2.14.2-882-gfe33df219d\n\n"},{"id":"329520","messageId":"20171003040737.2336-2-gitster@pobox.com","threadId":"46845","inReplyTo":"20171003040737.2336-1-gitster@pobox.com","subject":"[PATCH 1/2] Revert \"color: check color.ui in git_default_config()\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T04:07:36Z","receivedAt":"2017-10-03T04:07:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7.\n\nEven though we do want to fix the fallout from 4c7f1819 (\"make\ncolor.ui default to 'auto'\", 2013-06-10), which made it impossible\nto override it with \"git -c color.ui={never,always} $plumbing\",\nallowing the plumbing commands to pay attention to color.ui\nconfiguration variable turned out to be an unsatisfactory fix.\n\nPeople who had color.ui=always, thinking that it should be safe to\ndo, because it won't apply to plumbing commands, got burned by it.\n\nA bit of fix-up patches are needed, as the series that included the\npatch being reverted, and changes after the series landed, have\nand/or added code that assumes git_default_config() would read the\ncolor.ui, and they need to be adjusted.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/branch.c      | 2 +-\n builtin/clean.c       | 3 ++-\n builtin/grep.c        | 2 +-\n builtin/show-branch.c | 2 +-\n color.c               | 8 ++++++++\n config.c              | 4 ----\n diff.c                | 3 +++\n 7 files changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 16d391b407..1969c7116c 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -92,7 +92,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)\n \t\t\treturn config_error_nonbool(var);\n \t\treturn color_parse(value, branch_colors[slot]);\n \t}\n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static const char *branch_get_color(enum color_branch ix)\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex c1bafda5b6..057fc97fe4 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -125,7 +125,8 @@ static int git_clean_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n-\treturn git_default_config(var, value, cb);\n+\t/* inspect the color.ui config variable and others */\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static const char *clean_get_color(enum color_clean ix)\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex a7157f5632..0d6e669732 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -284,7 +284,7 @@ static int wait_all(void)\n static int grep_cmd_config(const char *var, const char *value, void *cb)\n {\n \tint st = grep_config(var, value, cb);\n-\tif (git_default_config(var, value, cb) < 0)\n+\tif (git_color_default_config(var, value, cb) < 0)\n \t\tst = -1;\n \n \tif (!strcmp(var, \"grep.threads\")) {\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 28f245c8cc..7073a3eb97 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -554,7 +554,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static int omit_in_dense(struct commit *commit, struct commit **rev, int n)\ndiff --git a/color.c b/color.c\nindex 7aa8b076f0..31b6207a00 100644\n--- a/color.c\n+++ b/color.c\n@@ -361,6 +361,14 @@ int git_color_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n+int git_color_default_config(const char *var, const char *value, void *cb)\n+{\n+\tif (git_color_config(var, value, cb) < 0)\n+\t\treturn -1;\n+\n+\treturn git_default_config(var, value, cb);\n+}\n+\n void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)\n {\n \tif (*color)\ndiff --git a/config.c b/config.c\nindex bc290e7563..a9356c1383 100644\n--- a/config.c\n+++ b/config.c\n@@ -16,7 +16,6 @@\n #include \"string-list.h\"\n #include \"utf8.h\"\n #include \"dir.h\"\n-#include \"color.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -1351,9 +1350,6 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \tif (starts_with(var, \"advice.\"))\n \t\treturn git_default_advice_config(var, value);\n \n-\tif (git_color_config(var, value, dummy) < 0)\n-\t\treturn -1;\n-\n \tif (!strcmp(var, \"pager.color\") || !strcmp(var, \"color.pager\")) {\n \t\tpager_use_color = git_config_bool(var,value);\n \t\treturn 0;\ndiff --git a/diff.c b/diff.c\nindex 9c38258030..85e714f6c6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -299,6 +299,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (git_color_config(var, value, cb) < 0)\n+\t\treturn -1;\n+\n \treturn git_diff_basic_config(var, value, cb);\n }\n \n-- \n2.14.2-882-gfe33df219d\n\n"},{"id":"329521","messageId":"20171003040737.2336-3-gitster@pobox.com","threadId":"46845","inReplyTo":"20171003040737.2336-1-gitster@pobox.com","subject":"[PATCH 2/2] colors: git_default_config() does not read color.ui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T04:07:37Z","receivedAt":"2017-10-03T04:07:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As we reverted 136c8c8b (\"color: check color.ui in\ngit_default_config()\", 2017-07-13), these need to be added back to\nthe codebase so that \"git tag --list\" and \"git for-each-ref\" would\nstill pay attention to color.ui setting.\n\nA real fix to the problem introduced by 4c7f1819 (\"make color.ui\ndefault to 'auto'\", 2013-06-10) must be found, to allow users to\noverride the default \"auto\" use of colors for any color-capable\nplumbing commands, but let's leave that to a later round.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/for-each-ref.c | 3 ++-\n builtin/tag.c          | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 5d7c921a77..238eb00e09 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -5,6 +5,7 @@\n #include \"object.h\"\n #include \"parse-options.h\"\n #include \"ref-filter.h\"\n+#include \"color.h\"\n \n static char const * const for_each_ref_usage[] = {\n \tN_(\"git for-each-ref [<options>] [<pattern>]\"),\n@@ -54,7 +55,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \n \tformat.format = \"%(objectname) %(objecttype)\\t%(refname)\";\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(git_color_default_config, NULL);\n \n \tparse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);\n \tif (maxcount < 0) {\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 66e35b823b..46c3e78b55 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -158,7 +158,7 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static void write_tag_body(int fd, const struct object_id *oid)\n-- \n2.14.2-882-gfe33df219d\n\n"},{"id":"329527","messageId":"20171003061515.72l35fpvcf63vsf6@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqqinfxklw7.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T06:15:16Z","receivedAt":"2017-10-03T06:15:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 11:25:44AM +0900, Junio C Hamano wrote:\n\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Jonathan Nieder <jrnieder@gmail.com> writes:\n> >\n> >> Yet another alternative would be to treat color.ui=always as a\n> >> deprecated synonym for color.ui=auto.  I think that's my preferred\n> >> fix.\n> >\n> > It is mine, too.  And I do not think color.ui=absolutely-always for\n> > those who want to keep the current behaviour is unneeded.\n> \n> Having said that, I do not mind solving what 136c8c8b (\"color: check\n> color.ui in git_default_config()\", 2017-07-13) tried to do in a\n> different way.  If 4c7f1819b that made even plumbing to default to\n> auto was wrong (because plumbing did not pay attention to color.ui\n> so people could not override that 'auto' default), we can partially\n> revert 4c7f1819 (\"make color.ui default to 'auto'\", 2013-06-10) to\n> make the default 'auto' not apply to plumbing.\n> \n> In any case, I think the first step may be to revert 136c8c8b from\n> both 'master' and 2.14.x.  These alternative solutions can come on\n> top.\n> \n> Thoughts?\n\nI'd prefer not to revert. I think setting any of the color config to\n\"always\" in an on-disk file is basically a broken config. It was\nexacerbated by 4c7f1819b, but it was already broken for scripts that\ncall \"git log\" or \"git diff\", or even just something as simple as piping\nthose programs on the command line.\n\nThe two reasonable paths forward to me are:\n\n  1. Do nothing. Putting \"color.ui = always\" in your on-disk config is a\n     bad idea, and the right fix is to stop doing it.\n\n  2. Make \"always\" a synonym for \"auto\". This has the advantage over (1)\n     that you can't shoot yourself in the foot with it (so the expanded\n     foot-shooting capabilities of 4c7f1819b don't matter). The\n     disadvantage is that you can no longer do:\n\n       git -c color.ui=always foo >not-a-tty\n\n     to ask for color in all sub-programs of \"foo\". I have no idea if\n     anybody cares. I came up with that example in 4c7f1819b as the most\n     plausible reason somebody might actually care about \"always\", but\n     I've never used it myself.\n\n     And there _is_ a way to get the same thing, which is:\n\n       GIT_PAGER_IN_USE=1 git foo >not-a-tty\n\n     I.e., stay in \"auto\" but make the claim that \"auto\" really ought to\n     be showing color because the output is going to be consumed\n     eventually by a human.  While it looks a bit funny in my made-up\n     example above (because the variables says \"pager\" but we're not\n     actually piping to a pager directly), I think this is the most\n     plausible reason that an actual program might want to override the\n     auto-color decision.\n\n-Peff\n"},{"id":"329536","messageId":"xmqqinfwiu5n.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171003061515.72l35fpvcf63vsf6@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T07:10:12Z","receivedAt":"2017-10-03T07:10:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'd prefer not to revert. I think setting any of the color config to\n> \"always\" in an on-disk file is basically a broken config. It was\n> exacerbated by 4c7f1819b, but it was already broken for scripts that\n> call \"git log\" or \"git diff\", or even just something as simple as piping\n> those programs on the command line.\n\nI actually disagree with that reasoning.\n\nWe've promised that plumbing commands were safe to use in scripts,\nand 4c7f1819 (\"make color.ui default to 'auto'\", 2013-06-10) got it\ncloser to breaking it (but not quite), and 136c8c8b finally broke\nit.  Setting ui.color=always and shooting themselves in the foot by\nseeing ANSI escapes in \"git log >file\" output is totally user's\nchoice.  Breaking scripts that carefully chose to use plumbing,\nbelieving our earlier promise, and blaming user's ui.color=always\ndoes not sound quite like the same thing, at least to me.\n"},{"id":"329537","messageId":"20171003071622.el6zqibfyrjc2mra@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqqinfwiu5n.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T07:16:22Z","receivedAt":"2017-10-03T07:16:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 04:10:12PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I'd prefer not to revert. I think setting any of the color config to\n> > \"always\" in an on-disk file is basically a broken config. It was\n> > exacerbated by 4c7f1819b, but it was already broken for scripts that\n> > call \"git log\" or \"git diff\", or even just something as simple as piping\n> > those programs on the command line.\n> \n> I actually disagree with that reasoning.\n> \n> We've promised that plumbing commands were safe to use in scripts,\n> and 4c7f1819 (\"make color.ui default to 'auto'\", 2013-06-10) got it\n> closer to breaking it (but not quite), and 136c8c8b finally broke\n> it.  Setting ui.color=always and shooting themselves in the foot by\n> seeing ANSI escapes in \"git log >file\" output is totally user's\n> choice.  Breaking scripts that carefully chose to use plumbing,\n> believing our earlier promise, and blaming user's ui.color=always\n> does not sound quite like the same thing, at least to me.\n\nI agree it's not quite the same thing, and I agree the problem was made\nmuch worse by 4c7f1819b.  But I still think color.ui=always is\ninherently a foot-gun, and in either case it is the user that sets it\nthat is harmed (and they are the ones who have the power to fix it).\n\nAt any rate, I think you only need to agree with that reason to buy into\nmy \"do nothing\" option.\n\nOption 2 would make everybody happy, I think (I'm working up a patch,\nthough note that it isn't quite trivial because of some abuses of the\ncolor system in the test scripts).\n\n-Peff\n"},{"id":"329538","messageId":"xmqqefqkiq8v.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171003071622.el6zqibfyrjc2mra@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T08:34:40Z","receivedAt":"2017-10-03T08:34:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I agree it's not quite the same thing, and I agree the problem was made\n> much worse by 4c7f1819b.  But I still think color.ui=always is\n> inherently a foot-gun, and in either case it is the user that sets it\n> that is harmed (and they are the ones who have the power to fix it).\n\nYeah, but it is inherently a foot-gun only for those who write\nscripts around porcelain commands, which are expected to honor\ncolor.ui=always.  If you write a script using the plumbing commands\nbecause you did not want to get broken by color.ui=always, and then\nyour script gets broken because plumbing commands you relied on\nsuddenly start paying attention to color.ui---is that the user's\nfault who used color.ui?  \n\nThe end-users have the power to work the breakage around by not\nusing \"always\".  The script writers have the power to work the\nbreakage around with \"--no-color\".  But these are workarounds that\nshouldn't have been needed in the first place, no?\n"},{"id":"329539","messageId":"20171003084506.lwmnrym4oyf66icz@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqqefqkiq8v.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T08:45:06Z","receivedAt":"2017-10-03T08:45:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 05:34:40PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I agree it's not quite the same thing, and I agree the problem was made\n> > much worse by 4c7f1819b.  But I still think color.ui=always is\n> > inherently a foot-gun, and in either case it is the user that sets it\n> > that is harmed (and they are the ones who have the power to fix it).\n> \n> Yeah, but it is inherently a foot-gun only for those who write\n> scripts around porcelain commands, which are expected to honor\n> color.ui=always.  If you write a script using the plumbing commands\n> because you did not want to get broken by color.ui=always, and then\n> your script gets broken because plumbing commands you relied on\n> suddenly start paying attention to color.ui---is that the user's\n> fault who used color.ui?\n\nNote that I'm arguing that it's a foot-gun even without scripts in the\npicture at all. Forget about plumbing versus porcelain. If you set\ncolor.ui to \"always\", you're going to get unexpected and confusing\nresults from time to time.\n\n> The end-users have the power to work the breakage around by not\n> using \"always\".  The script writers have the power to work the\n> breakage around with \"--no-color\".  But these are workarounds that\n> shouldn't have been needed in the first place, no?\n\nTo be clear, I don't think script writers should work around it at all.\nIt is either the fault of the user for having a broken config, or Git\nfor allowing such a broken config (and after having studied the\ndocumentation, I think Git really is not helping here; nobody should\never use \"always\", but the documentation introduces it as the most\nobvious option to choose).\n\n-Peff\n"},{"id":"329541","messageId":"xmqq4lrgip7u.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171003084506.lwmnrym4oyf66icz@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T08:56:53Z","receivedAt":"2017-10-03T08:57:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Note that I'm arguing that it's a foot-gun even without scripts in the\n> picture at all. Forget about plumbing versus porcelain. If you set\n> color.ui to \"always\", you're going to get unexpected and confusing\n> results from time to time.\n\nReally? \n\nI would think you would consistently get ANSI colored output in any\nmedium, even in files that you would later \"cat\" or \"less -R\" to\nview.  Is that unexpected?  Those who set \"always\" (I am not among\nthem, of course) would expect that, I would think.\n"},{"id":"329542","messageId":"20171003091049.jb46pyi3obtwkzsa@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqq4lrgip7u.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T09:10:50Z","receivedAt":"2017-10-03T09:10:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 05:56:53PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Note that I'm arguing that it's a foot-gun even without scripts in the\n> > picture at all. Forget about plumbing versus porcelain. If you set\n> > color.ui to \"always\", you're going to get unexpected and confusing\n> > results from time to time.\n> \n> Really? \n> \n> I would think you would consistently get ANSI colored output in any\n> medium, even in files that you would later \"cat\" or \"less -R\" to\n> view.  Is that unexpected?  Those who set \"always\" (I am not among\n> them, of course) would expect that, I would think.\n\nThose cases might be expected. But color when piping to grep or sed are\nnot. I guess you can lump those under \"well, they should be using\nplumbing, of course\" but I don't think that's very realistic. People do\nad-hoc pipes in their shells all the time (well, I assume so; I\ncertainly do).\n\nI don't argue that people don't have a need to write colors to a\nnon-terminal. Certainly they do. I argue that setting it in your on-disk\nconfig is likely to have it trigger at other times when it's unexpected\nand inconvenient.\n\nI dunno. Maybe I am wrong, because certainly I never set it. We've had\ntwo reports on the list since v2.14.2. The motivation for the first was\n\"I have no idea why I set that, and I'll switch to auto\". This is the\nsecond.\n\n-Peff\n"},{"id":"329543","messageId":"CAB2yay-0Z6xsL6Sp-428UHmxaUnMHObT8qb-UFG1TcMu_E8gPg@mail.gmail.com","threadId":"46845","inReplyTo":"20171003091049.jb46pyi3obtwkzsa@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Tsvi Mostovicz","fromEmail":"ttmost@gmail.com","sentAt":"2017-10-03T09:18:59Z","receivedAt":"2017-10-03T09:19:26Z","isPatch":false,"sender":{"key":"ttmost@gmail.com","avatar":null},"body":"I also don't remember why I set it, and as such I removed it.\n\nA helpful hint as to a bad config option would've been great.\nSomething along the lines of \"The use of color.ui = always is not\nrecommended\", with a flag allowing you to disable said warning.\n\nThanks,\n\nTsvi\nTsvi Mostovicz\nttmost@gmail.com\nwww.linkedin.com/in/tsvim\n\n\nOn Tue, Oct 3, 2017 at 12:10 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Oct 03, 2017 at 05:56:53PM +0900, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>>\n>> > Note that I'm arguing that it's a foot-gun even without scripts in the\n>> > picture at all. Forget about plumbing versus porcelain. If you set\n>> > color.ui to \"always\", you're going to get unexpected and confusing\n>> > results from time to time.\n>>\n>> Really?\n>>\n>> I would think you would consistently get ANSI colored output in any\n>> medium, even in files that you would later \"cat\" or \"less -R\" to\n>> view.  Is that unexpected?  Those who set \"always\" (I am not among\n>> them, of course) would expect that, I would think.\n>\n> Those cases might be expected. But color when piping to grep or sed are\n> not. I guess you can lump those under \"well, they should be using\n> plumbing, of course\" but I don't think that's very realistic. People do\n> ad-hoc pipes in their shells all the time (well, I assume so; I\n> certainly do).\n>\n> I don't argue that people don't have a need to write colors to a\n> non-terminal. Certainly they do. I argue that setting it in your on-disk\n> config is likely to have it trigger at other times when it's unexpected\n> and inconvenient.\n>\n> I dunno. Maybe I am wrong, because certainly I never set it. We've had\n> two reports on the list since v2.14.2. The motivation for the first was\n> \"I have no idea why I set that, and I'll switch to auto\". This is the\n> second.\n>\n> -Peff\n"},{"id":"329544","messageId":"20171003093157.gq7za2fwcqsouire@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003061515.72l35fpvcf63vsf6@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T09:31:58Z","receivedAt":"2017-10-03T09:32:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 02:15:15AM -0400, Jeff King wrote:\n\n> The two reasonable paths forward to me are:\n> \n>   1. Do nothing. Putting \"color.ui = always\" in your on-disk config is a\n>      bad idea, and the right fix is to stop doing it.\n> \n>   2. Make \"always\" a synonym for \"auto\". This has the advantage over (1)\n>      that you can't shoot yourself in the foot with it (so the expanded\n>      foot-shooting capabilities of 4c7f1819b don't matter). The\n>      disadvantage is that you can no longer do:\n> \n>        git -c color.ui=always foo >not-a-tty\n> \n>      to ask for color in all sub-programs of \"foo\". I have no idea if\n>      anybody cares. I came up with that example in 4c7f1819b as the most\n>      plausible reason somebody might actually care about \"always\", but\n>      I've never used it myself.\n> \n>      And there _is_ a way to get the same thing, which is:\n> \n>        GIT_PAGER_IN_USE=1 git foo >not-a-tty\n> \n>      I.e., stay in \"auto\" but make the claim that \"auto\" really ought to\n>      be showing color because the output is going to be consumed\n>      eventually by a human.  While it looks a bit funny in my made-up\n>      example above (because the variables says \"pager\" but we're not\n>      actually piping to a pager directly), I think this is the most\n>      plausible reason that an actual program might want to override the\n>      auto-color decision.\n\nI've got a series mostly done for option 2. It actually touches quite a\nfew of the test scripts, since we use \"-c color.ui=always\" to test\ncolor output. Normally having to touch the test suite a lot gives me\npause that we're breaking something for real users, but I think in this\ncase the test suite is doing something that normal users simply don't.\n\nA variant of 2 (let's call it 2a) is to convert \"always\" to \"auto\"\n_only_ for on-disk config, allowing \"-c\" to still work. This dull the\ntest-suite pain, but there are still several test scripts which use\ntest_config and need updating. And it introduces a weird inconsistency\nthat I'd just as soon skip.\n\nIf we buy into the idea that \"color.ui=always\" is actually a useful\nconstruct to put in your on-disk config (and I'm not sure that I do),\nthen our options are basically:\n\n  3. Go back to the earlier behavior, which i what Junio's 2-patch\n     revert series does. That has two issues though:\n\n       - after patch 2, for-each-ref still respects color.ui=always,\n\t even though it's plumbing. This is maybe-OK because we don't\n\t generate colors unless somebody puts %C() codes into their\n\t format.\n\n       - plumbing like diff-tree still doesn't respect color.ui=never\n\n  4. Read color.ui everywhere, but downgrade \"always\" to \"auto\" in\n     plumbing commands (which would have to mark themselves as such by\n     setting a global). This again falls into the \"inconsistent and hard\n     to explain\" category, though I think it at least behaves as people\n     would generally want. Except for people who really do want:\n\n       git -c color.always=true foo >file\n\n     to produce color when \"foo\" is a script that uses diff plumbing\n     under the hood. I don't think that _can_ be solved, since from the\n     plumbing perspective this is the same as the buggy add--interactive\n     case.\n\nI still lean towards option 2 (kill off \"always\"), but I'm trying to\nconsider the full range of options.\n\n-Peff\n"},{"id":"329550","messageId":"xmqqpoa4h5y7.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171003091049.jb46pyi3obtwkzsa@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-03T10:38:24Z","receivedAt":"2017-10-03T10:38:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Oct 03, 2017 at 05:56:53PM +0900, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > Note that I'm arguing that it's a foot-gun even without scripts in the\n>> > picture at all. Forget about plumbing versus porcelain. If you set\n>> > color.ui to \"always\", you're going to get unexpected and confusing\n>> > results from time to time.\n>> \n>> Really? \n>> \n>> I would think you would consistently get ANSI colored output in any\n>> medium, even in files that you would later \"cat\" or \"less -R\" to\n>> view.  Is that unexpected?  Those who set \"always\" (I am not among\n>> them, of course) would expect that, I would think.\n>\n> Those cases might be expected. But color when piping to grep or sed are\n> not. I guess you can lump those under \"well, they should be using\n> plumbing, of course\" but I don't think that's very realistic. People do\n> ad-hoc pipes in their shells all the time (well, I assume so; I\n> certainly do).\n\nThat's an argument to say color.ui=always is not a useful thing to\nuse and Git is wrong to offer such a nonsense option.  I agree with\nboth of them.\n\nBut it is a different matter that plumbing commands are now doing\nthe \"color\" thing without being asked.  Reading the configuration\nthat is meant for human interaction adds insult to injury.  I think\nthe earlier \"everybody is colored by default\" that forgot to make\nsure the change does not affect plumbing was the main culprit, and\nwe were merely lucky that thanks to the auto-detection of \"auto\" we\ndid not break scripts.\n\nHaving said all that, unless we revert the entire series (and\npossibly other things that happened after the series was merged on\n'master' that assumes that now default_config would read the\ncolor.ui thing), we won't be able to get back to the state before\nthe \"add -p\" regression, it seems.  As -rc freeze period for the\nnext cycle is approaching fast, I wanted to make sure that we have a\nplan to address the regression (which does not have to solve what\nthe reverted commit tried to solve).  If you think we can get a\nworkable code in 2.14.x and 'master' that essentially castrates\n\"always\" and makes it the same as \"auto\" in several days tops, then\nI'd prefer such a solution, as honoring \"color.ui=always\" does not\nfeel all that important.\n"},{"id":"329559","messageId":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqqpoa4h5y7.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:37:13Z","receivedAt":"2017-10-03T13:37:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 03, 2017 at 07:38:24PM +0900, Junio C Hamano wrote:\n\n> That's an argument to say color.ui=always is not a useful thing to\n> use and Git is wrong to offer such a nonsense option.  I agree with\n> both of them.\n> \n> But it is a different matter that plumbing commands are now doing\n> the \"color\" thing without being asked.  Reading the configuration\n> that is meant for human interaction adds insult to injury.  I think\n> the earlier \"everybody is colored by default\" that forgot to make\n> sure the change does not affect plumbing was the main culprit, and\n> we were merely lucky that thanks to the auto-detection of \"auto\" we\n> did not break scripts.\n\nYes, I agree that the \"everybody is colored by default\" is the root of\nthe problem. And in that sense all of this discussion is band-aid fixes\non that.\n\nAt the same time, I have a suspicion that \"even plumbing is color=auto\nby default\" may actually be _helping_ in many scripts. Because they to\nuse the plumbing to show output to the user, respecting the user's\nnormal color preferences. That's just a hunch, though.\n\nI also think trying to revert that \"default\" patch (4c7f1819) may be a\npretty big pain at this point.\n\n  Side note: Sorry, I noticed while writing this that I got my commit\n  references mixed up earlier between 4c7f1819 (which turned the default\n  to auto even for plumbing) and 136c8c8b, my recent change to parse\n  color.ui in more places. When I said the problem was exacerbated/made\n  worse by 4c7f1819, I really meant 136c8c8b. Hopefully that didn't\n  confuse the discussion too much.\n\n> Having said all that, unless we revert the entire series (and\n> possibly other things that happened after the series was merged on\n> 'master' that assumes that now default_config would read the\n> color.ui thing), we won't be able to get back to the state before\n> the \"add -p\" regression, it seems.  As -rc freeze period for the\n> next cycle is approaching fast, I wanted to make sure that we have a\n> plan to address the regression (which does not have to solve what\n> the reverted commit tried to solve).  If you think we can get a\n> workable code in 2.14.x and 'master' that essentially castrates\n> \"always\" and makes it the same as \"auto\" in several days tops, then\n> I'd prefer such a solution, as honoring \"color.ui=always\" does not\n> feel all that important.\n\nHere's what I came up with on the \"color.ui=always is nonsense that we\nshould not offer\" front. The number of patches may be a little daunting,\nbut most of them are just removing cases of \"git -c color.ui=always\"\nfrom the tests.\n\n  [01/12]: test-terminal: set TERM=vt100\n  [02/12]: t4015: prefer --color to -c color.diff=always\n  [03/12]: t4015: use --color with --color-moved\n  [04/12]: t3701: use test-terminal to collect color output\n  [05/12]: t7508: use test_terminal for color output\n  [06/12]: t7502: use diff.noprefix for --verbose test\n  [07/12]: t6006: drop \"always\" color config tests\n  [08/12]: t3203: drop \"always\" color test\n  [09/12]: t7301: use test_terminal to check color\n  [10/12]: t3205: use --color instead of color.branch=always\n  [11/12]: provide --color option for all ref-filter users\n  [12/12]: color: make \"always\" the same as \"auto\" in config\n\n Documentation/config.txt           | 35 ++++++++++++-------------\n Documentation/git-for-each-ref.txt |  5 ++++\n Documentation/git-tag.txt          |  5 ++++\n builtin/for-each-ref.c             |  1 +\n builtin/tag.c                      |  1 +\n color.c                            |  2 +-\n t/t3203-branch-output.sh           |  8 +-----\n t/t3205-branch-color.sh            |  5 ++--\n t/t3701-add-interactive.sh         | 18 +++++++++----\n t/t4015-diff-whitespace.sh         | 53 +++++++++++++++++++-------------------\n t/t4202-log.sh                     |  2 +-\n t/t6006-rev-list-format.sh         | 23 +++++------------\n t/t6300-for-each-ref.sh            |  7 +++--\n t/t7004-tag.sh                     |  6 ++---\n t/t7006-pager.sh                   |  6 ++---\n t/t7301-clean-interactive.sh       |  5 ++--\n t/t7502-commit.sh                  |  4 +--\n t/t7508-status.sh                  | 41 +++++++++++++++--------------\n t/test-terminal.perl               |  1 +\n 19 files changed, 115 insertions(+), 113 deletions(-)\n\n-Peff\n"},{"id":"329560","messageId":"20171003133934.oo2xkcxvweh36tcs@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 01/12] test-terminal: set TERM=vt100","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:39:34Z","receivedAt":"2017-10-03T13:39:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of the test-terminal script is to simulate in the\ntest scripts an environment where output is going to a real\nterminal.\n\nBut since test-lib.sh also sets TERM=dumb, the simulation\nisn't very realistic. The color code will skip auto-coloring\nfor TERM=dumb, leading to us liberally sprinkling\n\n  test_terminal env TERM=vt100 git ...\n\nthrough the test suite to convince the tests to actually\ngenerate colors. Let's set TERM for programs run under\ntest_terminal, which is one less thing for test-writers to\nremember.\n\nIn most cases the callers can be simplified, but note there\nis one interesting case in t4202. It uses test_terminal to\ncheck the auto-enabling of --decorate, but the expected\noutput _doesn't_ contain colors (because TERM=dumb\nsuppresses them). Using TERM=vt100 is closer to what the\nreal world looks like; adjust the expected output to match.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNot strictly necessary for this series, but after banging my shins on\nthis nuisance for the umpteenth time, I decided to finally do something\nabout it.\n\n t/t3203-branch-output.sh   | 2 +-\n t/t4202-log.sh             | 2 +-\n t/t6006-rev-list-format.sh | 3 +--\n t/t6300-for-each-ref.sh    | 3 +--\n t/t7004-tag.sh             | 2 +-\n t/t7006-pager.sh           | 6 +++---\n t/test-terminal.perl       | 1 +\n 7 files changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex d2aec0f38b..86286f263d 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -253,7 +253,7 @@ test_expect_success '%(color) omitted without tty' '\n '\n \n test_expect_success TTY '%(color) present with tty' '\n-\ttest_terminal env TERM=vt100 git branch $color_args >actual.raw &&\n+\ttest_terminal git branch $color_args >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expect.color actual\n '\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 36d120c969..8f155da7a5 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -750,7 +750,7 @@ test_expect_success 'log.decorate config parsing' '\n '\n \n test_expect_success TTY 'log output on a TTY' '\n-\tgit log --oneline --decorate >expect.short &&\n+\tgit log --color --oneline --decorate >expect.short &&\n \n \ttest_terminal git log --oneline >actual &&\n \ttest_cmp expect.short actual\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex b326d550f3..98be78b4a2 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -229,8 +229,7 @@ do\n \t'\n \n \ttest_expect_success TTY \"$desc respects --color=auto (stdout is tty)\" '\n-\t\ttest_terminal env TERM=vt100 \\\n-\t\t\tgit log --format=$color -1 --color=auto >actual &&\n+\t\ttest_terminal git log --format=$color -1 --color=auto >actual &&\n \t\thas_color actual\n \t'\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2274a4b733..d6bffe6273 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -425,8 +425,7 @@ test_expect_success 'set up color tests' '\n '\n \n test_expect_success TTY '%(color) shows color with a tty' '\n-\ttest_terminal env TERM=vt100 \\\n-\t\tgit for-each-ref --format=\"$color_format\" >actual.raw &&\n+\ttest_terminal git for-each-ref --format=\"$color_format\" >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expected.color actual\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b545c33f83..0a86f8ea39 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1907,7 +1907,7 @@ test_expect_success '%(color) omitted without tty' '\n '\n \n test_expect_success TTY '%(color) present with tty' '\n-\ttest_terminal env TERM=vt100 git tag $color_args >actual.raw &&\n+\ttest_terminal git tag $color_args >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expect.color actual\n '\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 9128ec5acd..f0f1abd1c2 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -239,7 +239,7 @@ test_expect_success 'no color when stdout is a regular file' '\n test_expect_success TTY 'color when writing to a pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n-\ttest_terminal env TERM=vt100 git log &&\n+\ttest_terminal git log &&\n \tcolorful paginated.out\n '\n \n@@ -247,7 +247,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n \ttest_config color.pager false &&\n-\ttest_terminal env TERM=vt100 git log &&\n+\ttest_terminal git log &&\n \t! colorful paginated.out\n '\n \n@@ -266,7 +266,7 @@ test_expect_success 'color when writing to a file intended for a pager' '\n test_expect_success TTY 'colors are sent to pager for external commands' '\n \ttest_config alias.externallog \"!git log\" &&\n \ttest_config color.ui auto &&\n-\ttest_terminal env TERM=vt100 git -p externallog &&\n+\ttest_terminal git -p externallog &&\n \tcolorful paginated.out\n '\n \ndiff --git a/t/test-terminal.perl b/t/test-terminal.perl\nindex 96b6a03e1c..46bf618479 100755\n--- a/t/test-terminal.perl\n+++ b/t/test-terminal.perl\n@@ -80,6 +80,7 @@ sub copy_stdio {\n if ($#ARGV < 1) {\n \tdie \"usage: test-terminal program args\";\n }\n+$ENV{TERM} = 'vt100';\n my $master_in = new IO::Pty;\n my $master_out = new IO::Pty;\n my $master_err = new IO::Pty;\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329561","messageId":"20171003134019.3hmpjpkbm4fes36w@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 02/12] t4015: prefer --color to -c color.diff=always","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:40:19Z","receivedAt":"2017-10-03T13:40:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"t4015 contains many color-related tests which need to\noverride the \"is stdout a tty\" check. They do so by setting\nthe color.diff config, but we can accomplish the same with\nthe --color option. Besides being shorter to type, switching\nwill prepare us for upcoming changes to \"always\" when see it\nin config.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t4015-diff-whitespace.sh | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 12d182dc1b..b9df886e37 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -821,7 +821,7 @@ test_expect_success 'diff that introduces a line with only tabs' '\n \techo \"test\" >x &&\n \tgit commit -m \"initial\" x &&\n \techo \"{NTN}\" | tr \"NT\" \"\\n\\t\" >>x &&\n-\tgit -c color.diff=always diff | test_decode_color >current &&\n+\tgit diff --color | test_decode_color >current &&\n \n \tcat >expected <<-\\EOF &&\n \t<BOLD>diff --git a/x b/x<RESET>\n@@ -851,7 +851,7 @@ test_expect_success 'diff that introduces and removes ws breakages' '\n \t\techo \"2. and a new line \"\n \t} >x &&\n \n-\tgit -c color.diff=always diff |\n+\tgit diff --color |\n \ttest_decode_color >current &&\n \n \tcat >expected <<-\\EOF &&\n@@ -923,15 +923,15 @@ test_expect_success 'ws-error-highlight test setup' '\n \n test_expect_success 'test --ws-error-highlight option' '\n \n-\tgit -c color.diff=always diff --ws-error-highlight=default,old |\n+\tgit diff --color --ws-error-highlight=default,old |\n \ttest_decode_color >current &&\n \ttest_cmp expect.default-old current &&\n \n-\tgit -c color.diff=always diff --ws-error-highlight=all |\n+\tgit diff --color --ws-error-highlight=all |\n \ttest_decode_color >current &&\n \ttest_cmp expect.all current &&\n \n-\tgit -c color.diff=always diff --ws-error-highlight=none |\n+\tgit diff --color --ws-error-highlight=none |\n \ttest_decode_color >current &&\n \ttest_cmp expect.none current\n \n@@ -939,15 +939,15 @@ test_expect_success 'test --ws-error-highlight option' '\n \n test_expect_success 'test diff.wsErrorHighlight config' '\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=default,old diff |\n+\tgit -c diff.wsErrorHighlight=default,old diff --color |\n \ttest_decode_color >current &&\n \ttest_cmp expect.default-old current &&\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=all diff |\n+\tgit -c diff.wsErrorHighlight=all diff --color |\n \ttest_decode_color >current &&\n \ttest_cmp expect.all current &&\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=none diff |\n+\tgit -c diff.wsErrorHighlight=none diff --color |\n \ttest_decode_color >current &&\n \ttest_cmp expect.none current\n \n@@ -955,18 +955,18 @@ test_expect_success 'test diff.wsErrorHighlight config' '\n \n test_expect_success 'option overrides diff.wsErrorHighlight' '\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=none \\\n-\t\tdiff --ws-error-highlight=default,old |\n+\tgit -c diff.wsErrorHighlight=none \\\n+\t\tdiff --color --ws-error-highlight=default,old |\n \ttest_decode_color >current &&\n \ttest_cmp expect.default-old current &&\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=default \\\n-\t\tdiff --ws-error-highlight=all |\n+\tgit -c diff.wsErrorHighlight=default \\\n+\t\tdiff --color --ws-error-highlight=all |\n \ttest_decode_color >current &&\n \ttest_cmp expect.all current &&\n \n-\tgit -c color.diff=always -c diff.wsErrorHighlight=all \\\n-\t\tdiff --ws-error-highlight=none |\n+\tgit -c diff.wsErrorHighlight=all \\\n+\t\tdiff --color --ws-error-highlight=none |\n \ttest_decode_color >current &&\n \ttest_cmp expect.none current\n \n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329562","messageId":"20171003134151.znylpiijmlmk2akx@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 03/12] t4015: use --color with --color-moved","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:41:51Z","receivedAt":"2017-10-03T13:41:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The tests for --color-moved write their output to a file,\nbut doing so suppresses color output under \"auto\". Right now\nthis is solved by running the whole script under\n\"color.diff=always\". In preparation for the behavior of\n\"always\" changing, let's explicitly enable color.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI kind of think `--color-moved` should imply `--color`, as\n`--color-words` seems to. But that seemed like a potential rabbit-hole,\nand this series is already contentious enough.\n\n t/t4015-diff-whitespace.sh | 25 ++++++++++++-------------\n 1 file changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex b9df886e37..3bca958863 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -802,7 +802,6 @@ test_expect_success 'combined diff with autocrlf conversion' '\n # Start testing the colored format for whitespace checks\n \n test_expect_success 'setup diff colors' '\n-\tgit config color.diff always &&\n \tgit config color.diff.plain normal &&\n \tgit config color.diff.meta bold &&\n \tgit config color.diff.frag cyan &&\n@@ -986,7 +985,7 @@ test_expect_success 'detect moved code, complete file' '\n \tgit mv test.c main.c &&\n \ttest_config color.diff.oldMoved \"normal red\" &&\n \ttest_config color.diff.newMoved \"normal green\" &&\n-\tgit diff HEAD --color-moved=zebra --no-renames | test_decode_color >actual &&\n+\tgit diff HEAD --color-moved=zebra --color --no-renames | test_decode_color >actual &&\n \tcat >expected <<-\\EOF &&\n \t<BOLD>diff --git a/main.c b/main.c<RESET>\n \t<BOLD>new file mode 100644<RESET>\n@@ -1087,7 +1086,7 @@ test_expect_success 'detect malicious moved code, inside file' '\n \t\t\tbar();\n \t\t}\n \tEOF\n-\tgit diff HEAD --no-renames --color-moved=zebra| test_decode_color >actual &&\n+\tgit diff HEAD --no-renames --color-moved=zebra --color | test_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n \t<BOLD>diff --git a/main.c b/main.c<RESET>\n \t<BOLD>index 27a619c..7cf9336 100644<RESET>\n@@ -1136,7 +1135,7 @@ test_expect_success 'plain moved code, inside file' '\n \ttest_config color.diff.oldMovedAlternative \"blue\" &&\n \ttest_config color.diff.newMovedAlternative \"yellow\" &&\n \t# needs previous test as setup\n-\tgit diff HEAD --no-renames --color-moved=plain| test_decode_color >actual &&\n+\tgit diff HEAD --no-renames --color-moved=plain --color | test_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n \t<BOLD>diff --git a/main.c b/main.c<RESET>\n \t<BOLD>index 27a619c..7cf9336 100644<RESET>\n@@ -1227,7 +1226,7 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '\n \ttest_config color.diff.newMovedDimmed \"normal cyan\" &&\n \ttest_config color.diff.oldMovedAlternativeDimmed \"normal blue\" &&\n \ttest_config color.diff.newMovedAlternativeDimmed \"normal yellow\" &&\n-\tgit diff HEAD --no-renames --color-moved=dimmed_zebra |\n+\tgit diff HEAD --no-renames --color-moved=dimmed_zebra --color |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n@@ -1271,7 +1270,7 @@ test_expect_success 'cmd option assumes configured colored-moved' '\n \ttest_config color.diff.oldMovedAlternativeDimmed \"normal blue\" &&\n \ttest_config color.diff.newMovedAlternativeDimmed \"normal yellow\" &&\n \ttest_config diff.colorMoved zebra &&\n-\tgit diff HEAD --no-renames --color-moved |\n+\tgit diff HEAD --no-renames --color-moved --color |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n@@ -1343,7 +1342,7 @@ line 4\n EOF\n \ttest_config color.diff.oldMoved \"magenta\" &&\n \ttest_config color.diff.newMoved \"cyan\" &&\n-\tgit diff HEAD --no-renames --color-moved |\n+\tgit diff HEAD --no-renames --color-moved --color |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n@@ -1364,7 +1363,7 @@ EOF\n \tEOF\n \ttest_cmp expected actual &&\n \n-\tgit diff HEAD --no-renames -w --color-moved |\n+\tgit diff HEAD --no-renames -w --color-moved --color |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat <<-\\EOF >expected &&\n@@ -1403,7 +1402,7 @@ test_expect_success '--color-moved block at end of diff output respects MIN_ALNU\n \tirrelevant_line\n \tEOF\n \n-\tgit diff HEAD --color-moved=zebra --no-renames |\n+\tgit diff HEAD --color-moved=zebra --color --no-renames |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat >expected <<-\\EOF &&\n@@ -1442,7 +1441,7 @@ test_expect_success '--color-moved respects MIN_ALNUM_COUNT' '\n \tnineteen chars 456789\n \tEOF\n \n-\tgit diff HEAD --color-moved=zebra --no-renames |\n+\tgit diff HEAD --color-moved=zebra --color --no-renames |\n \t\tgrep -v \"index\" |\n \t\ttest_decode_color >actual &&\n \tcat >expected <<-\\EOF &&\n@@ -1485,7 +1484,7 @@ test_expect_success '--color-moved treats adjacent blocks as separate for MIN_AL\n \t7charsA\n \tEOF\n \n-\tgit diff HEAD --color-moved=zebra --no-renames | grep -v \"index\" | test_decode_color >actual &&\n+\tgit diff HEAD --color-moved=zebra --color --no-renames | grep -v \"index\" | test_decode_color >actual &&\n \tcat >expected <<-\\EOF &&\n \t<BOLD>diff --git a/bar b/bar<RESET>\n \t<BOLD>--- a/bar<RESET>\n@@ -1519,7 +1518,7 @@ test_expect_success 'move detection with submodules' '\n \techo foul >bananas/recipe &&\n \techo ripe >fruit.t &&\n \n-\tgit diff --submodule=diff --color-moved >actual &&\n+\tgit diff --submodule=diff --color-moved --color >actual &&\n \n \t# no move detection as the moved line is across repository boundaries.\n \ttest_decode_color <actual >decoded_actual &&\n@@ -1527,7 +1526,7 @@ test_expect_success 'move detection with submodules' '\n \t! grep BRED decoded_actual &&\n \n \t# nor did we mess with it another way\n-\tgit diff --submodule=diff | test_decode_color >expect &&\n+\tgit diff --submodule=diff --color | test_decode_color >expect &&\n \ttest_cmp expect decoded_actual\n '\n \n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329563","messageId":"20171003134215.oce7bdemb7cjyg22@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 04/12] t3701: use test-terminal to collect color output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:42:15Z","receivedAt":"2017-10-03T13:42:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When testing whether \"add -p\" can generate colors, we set\ncolor.ui to \"always\". This isn't a very good test, as in the\nreal-world a user typically has \"auto\" coupled with stdout\ngoing to a terminal (and it's plausible that this could mask\na real bug in add--interactive if we depend on plumbing's\nisatty check).\n\nLet's switch to test_terminal, which gives us a more\nrealistic environment. This also prepare us for future\nchanges to the \"always\" color option.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t3701-add-interactive.sh | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 2f3e7cea64..39d0130f88 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -2,6 +2,7 @@\n \n test_description='add -i basic tests'\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n if ! test_have_prereq PERL\n then\n@@ -380,14 +381,11 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \ttest_cmp expected diff\n '\n \n-test_expect_success 'diffs can be colorized' '\n+test_expect_success TTY 'diffs can be colorized' '\n \tgit reset --hard &&\n \n-\t# force color even though the test script has no terminal\n-\ttest_config color.ui always &&\n-\n \techo content >test &&\n-\tprintf y | git add -p >output 2>&1 &&\n+\tprintf y | test_terminal git add -p >output 2>&1 &&\n \n \t# We do not want to depend on the exact coloring scheme\n \t# git uses for diffs, so just check that we saw some kind of color.\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329564","messageId":"20171003134329.wtgpwrvdpf2s6ls7@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 05/12] t7508: use test_terminal for color output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:43:29Z","receivedAt":"2017-10-03T13:43:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This script tests the output of status with various formats\nwhen color is enabled. It uses the \"always\" setting so that\nthe output is valid even though we capture it in a file.\nUsing test_terminal gives us a more realistic environment,\nand prepares us for the behavior of \"always\" changing.\n\nArguably we are testing less than before, since \"auto\" is\nalready the default, and we can no longer tell if the config\nis actually doing anything.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI noticed that \"status\" does not have a \"--color\" option. I think it\nmight be worth adding one for completeness, though I still prefer the\ntest_terminal solution here.\n\n t/t7508-status.sh | 41 +++++++++++++++++++++--------------------\n 1 file changed, 21 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 43d19a9b22..a3d760e63a 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -6,6 +6,7 @@\n test_description='git status'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n test_expect_success 'status -h in broken repository' '\n \tgit config --global advice.statusuoption false &&\n@@ -667,7 +668,7 @@ test_expect_success 'setup unique colors' '\n \n '\n \n-test_expect_success 'status with color.ui' '\n+test_expect_success TTY 'status with color.ui' '\n \tcat >expect <<\\EOF &&\n On branch <GREEN>master<RESET>\n Your branch and '\\''upstream'\\'' have diverged,\n@@ -694,14 +695,14 @@ Untracked files:\n \t<BLUE>untracked<RESET>\n \n EOF\n-\ttest_config color.ui always &&\n-\tgit status | test_decode_color >output &&\n+\ttest_config color.ui auto &&\n+\ttest_terminal git status | test_decode_color >output &&\n \ttest_i18ncmp expect output\n '\n \n-test_expect_success 'status with color.status' '\n-\ttest_config color.status always &&\n-\tgit status | test_decode_color >output &&\n+test_expect_success TTY 'status with color.status' '\n+\ttest_config color.status auto &&\n+\ttest_terminal git status | test_decode_color >output &&\n \ttest_i18ncmp expect output\n '\n \n@@ -714,19 +715,19 @@ cat >expect <<\\EOF\n <BLUE>??<RESET> untracked\n EOF\n \n-test_expect_success 'status -s with color.ui' '\n+test_expect_success TTY 'status -s with color.ui' '\n \n-\tgit config color.ui always &&\n-\tgit status -s | test_decode_color >output &&\n+\tgit config color.ui auto &&\n+\ttest_terminal git status -s | test_decode_color >output &&\n \ttest_cmp expect output\n \n '\n \n-test_expect_success 'status -s with color.status' '\n+test_expect_success TTY 'status -s with color.status' '\n \n \tgit config --unset color.ui &&\n-\tgit config color.status always &&\n-\tgit status -s | test_decode_color >output &&\n+\tgit config color.status auto &&\n+\ttest_terminal git status -s | test_decode_color >output &&\n \ttest_cmp expect output\n \n '\n@@ -741,9 +742,9 @@ cat >expect <<\\EOF\n <BLUE>??<RESET> untracked\n EOF\n \n-test_expect_success 'status -s -b with color.status' '\n+test_expect_success TTY 'status -s -b with color.status' '\n \n-\tgit status -s -b | test_decode_color >output &&\n+\ttest_terminal git status -s -b | test_decode_color >output &&\n \ttest_i18ncmp expect output\n \n '\n@@ -757,20 +758,20 @@ A  dir2/added\n ?? untracked\n EOF\n \n-test_expect_success 'status --porcelain ignores color.ui' '\n+test_expect_success TTY 'status --porcelain ignores color.ui' '\n \n \tgit config --unset color.status &&\n-\tgit config color.ui always &&\n-\tgit status --porcelain | test_decode_color >output &&\n+\tgit config color.ui auto &&\n+\ttest_terminal git status --porcelain | test_decode_color >output &&\n \ttest_cmp expect output\n \n '\n \n-test_expect_success 'status --porcelain ignores color.status' '\n+test_expect_success TTY 'status --porcelain ignores color.status' '\n \n \tgit config --unset color.ui &&\n-\tgit config color.status always &&\n-\tgit status --porcelain | test_decode_color >output &&\n+\tgit config color.status auto &&\n+\ttest_terminal git status --porcelain | test_decode_color >output &&\n \ttest_cmp expect output\n \n '\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329565","messageId":"20171003134347.v3nhmxhp4wr2em3k@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 06/12] t7502: use diff.noprefix for --verbose test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:43:47Z","receivedAt":"2017-10-03T13:43:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"To check that \"status -v\" respects diff config, we set\n\"color.diff\" and look at the output of \"status\". We could\nequally well use any diff config. Since color output depends\non a lot of other factors (like whether stdout is a tty, and\nhow we interpret \"always\"), let's use a more mundane option.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t7502-commit.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 725687d5d5..d33a3cb331 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -171,9 +171,9 @@ test_expect_success 'verbose' '\n \n test_expect_success 'verbose respects diff config' '\n \n-\ttest_config color.diff always &&\n+\ttest_config diff.noprefix true &&\n \tgit status -v >actual &&\n-\tgrep \"\\[1mdiff --git\" actual\n+\tgrep \"diff --git negative negative\" actual\n '\n \n mesg_with_comment_and_newlines='\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329566","messageId":"20171003134427.n7iy47yse5rydnfm@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 07/12] t6006: drop \"always\" color config tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:44:27Z","receivedAt":"2017-10-03T13:44:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We test the %C() format placeholders with a variety of\ncolor-inducing options, including \"--color\" and\n\"-c color.ui=always\". In preparation for the behavior of\n\"always\" changing, we need to do something with those\n\"always\" tests.\n\nWe can drop ones that expect \"always\" to turn on color even\nto a file, as that will become a synonym for \"auto\", which\nis already tested.\n\nFor the \"--no-color\" test, we need to make sure that color\nwould otherwise be shown. To do this, we can use\ntest_terminal, which enables colors in the default setup.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t6006-rev-list-format.sh | 20 +++++---------------\n 1 file changed, 5 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 98be78b4a2..25a9c65dc5 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -208,26 +208,11 @@ do\n \t\thas_no_color actual\n \t'\n \n-\ttest_expect_success \"$desc enables colors for color.diff\" '\n-\t\tgit -c color.diff=always log --format=$color -1 >actual &&\n-\t\thas_color actual\n-\t'\n-\n-\ttest_expect_success \"$desc enables colors for color.ui\" '\n-\t\tgit -c color.ui=always log --format=$color -1 >actual &&\n-\t\thas_color actual\n-\t'\n-\n \ttest_expect_success \"$desc respects --color\" '\n \t\tgit log --format=$color -1 --color >actual &&\n \t\thas_color actual\n \t'\n \n-\ttest_expect_success \"$desc respects --no-color\" '\n-\t\tgit -c color.ui=always log --format=$color -1 --no-color >actual &&\n-\t\thas_no_color actual\n-\t'\n-\n \ttest_expect_success TTY \"$desc respects --color=auto (stdout is tty)\" '\n \t\ttest_terminal git log --format=$color -1 --color=auto >actual &&\n \t\thas_color actual\n@@ -240,6 +225,11 @@ do\n \t\t\thas_no_color actual\n \t\t)\n \t'\n+\n+\ttest_expect_success TTY \"$desc respects --no-color\" '\n+\t\ttest_terminal git log --format=$color -1 --no-color >actual &&\n+\t\thas_no_color actual\n+\t'\n done\n \n test_expect_success '%C(always,...) enables color even without tty' '\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329567","messageId":"20171003134438.fmjc5nziuw24hnot@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 08/12] t3203: drop \"always\" color test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:44:39Z","receivedAt":"2017-10-03T13:44:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In preparation for the behavior of \"always\" changing to\nmatch \"auto\", we can simply drop this test. We already check\nother forms (like \"--color\") independently.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t3203-branch-output.sh | 6 ------\n 1 file changed, 6 deletions(-)\n\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex 86286f263d..ee6787614c 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -258,12 +258,6 @@ test_expect_success TTY '%(color) present with tty' '\n \ttest_cmp expect.color actual\n '\n \n-test_expect_success 'color.branch=always overrides auto-color' '\n-\tgit -c color.branch=always branch $color_args >actual.raw &&\n-\ttest_decode_color <actual.raw >actual &&\n-\ttest_cmp expect.color actual\n-'\n-\n test_expect_success '--color overrides auto-color' '\n \tgit branch --color $color_args >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329568","messageId":"20171003134458.fl7jnfz4s3tn3gm7@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 09/12] t7301: use test_terminal to check color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:44:58Z","receivedAt":"2017-10-03T13:45:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This test wants to confirm that \"clean -i\" shows color\noutput. Using test_terminal gives us a more realistic\nenvironment than \"color.ui=always\", and prepares us for the\nbehavior of \"always\" changing in a future patch.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t7301-clean-interactive.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh\nindex 556e1850e2..1bf9789c8a 100755\n--- a/t/t7301-clean-interactive.sh\n+++ b/t/t7301-clean-interactive.sh\n@@ -3,6 +3,7 @@\n test_description='git clean -i basic tests'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n test_expect_success 'setup' '\n \n@@ -472,10 +473,10 @@ test_expect_success 'git clean -id with prefix and path (ask)' '\n \n '\n \n-test_expect_success 'git clean -i paints the header in HEADER color' '\n+test_expect_success TTY 'git clean -i paints the header in HEADER color' '\n \t>a.out &&\n \techo q |\n-\tgit -c color.ui=always clean -i |\n+\ttest_terminal git clean -i |\n \ttest_decode_color |\n \thead -n 1 >header &&\n \t# not i18ngrep\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329569","messageId":"20171003134518.4egfo7oqpemo3a7q@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 10/12] t3205: use --color instead of color.branch=always","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:45:18Z","receivedAt":"2017-10-03T13:45:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"To test the color output, we must convince \"git branch\" to\nwrite colors to a non-terminal. We do that now by setting\nthe color config to \"always\".  In preparation for the\nbehavior of \"always\" changing, let's switch to using the\n\"--color\" command-line option, which is more direct.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t3205-branch-color.sh | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3205-branch-color.sh b/t/t3205-branch-color.sh\nindex 9343550f50..4f1e16bb44 100755\n--- a/t/t3205-branch-color.sh\n+++ b/t/t3205-branch-color.sh\n@@ -12,7 +12,6 @@ test_expect_success 'set up some sample branches' '\n # choose non-default colors to make sure config\n # is taking effect\n test_expect_success 'set up some color config' '\n-\tgit config color.branch always &&\n \tgit config color.branch.local blue &&\n \tgit config color.branch.remote yellow &&\n \tgit config color.branch.current cyan\n@@ -24,7 +23,7 @@ test_expect_success 'regular output shows colors' '\n \t  <BLUE>other<RESET>\n \t  <YELLOW>remotes/origin/master<RESET>\n \tEOF\n-\tgit branch -a >actual.raw &&\n+\tgit branch --color -a >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expect actual\n '\n@@ -36,7 +35,7 @@ test_expect_success 'verbose output shows colors' '\n \t  <BLUE>other                <RESET> $oid foo\n \t  <YELLOW>remotes/origin/master<RESET> $oid foo\n \tEOF\n-\tgit branch -v -a >actual.raw &&\n+\tgit branch --color -v -a >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329570","messageId":"20171003134547.c6o6kmwqhpk2eyq7@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 11/12] provide --color option for all ref-filter users","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:45:47Z","receivedAt":"2017-10-03T13:46:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When ref-filter learned about want_color() in 11b087adfd\n(ref-filter: consult want_color() before emitting colors,\n2017-07-13), it became useful to be able to turn colors off\nand on for specific commands. For git-branch, you can do so\nwith --color/--no-color.\n\nBut for git-for-each-ref and git-tag, the other users of\nref-filter, you have no option except to tweak the\n\"color.ui\" config setting. Let's give both of these commands\nthe usual color command-line options.\n\nThis is a bit more obvious as a method for overriding the\nconfig. And it also prepares us for the behavior of \"always\"\nchanging (so that we are still left with a way of forcing\ncolor when our output goes to a non-terminal).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-for-each-ref.txt | 5 +++++\n Documentation/git-tag.txt          | 5 +++++\n builtin/for-each-ref.c             | 1 +\n builtin/tag.c                      | 1 +\n t/t6300-for-each-ref.sh            | 4 ++--\n t/t7004-tag.sh                     | 4 ++--\n 6 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 66b4e0a405..cbd0a6212a 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -57,6 +57,11 @@ OPTIONS\n \t`xx`; for example `%00` interpolates to `\\0` (NUL),\n \t`%09` to `\\t` (TAB) and `%0a` to `\\n` (LF).\n \n+--color[=<when>]:\n+\tRespect any colors specified in the `--format` option. The\n+\t`<when>` field must be one of `always`, `never`, or `auto` (if\n+\t`<when>` is absent, behave as if `always` was given).\n+\n --shell::\n --perl::\n --python::\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 95e9f391d8..956fc019f9 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -115,6 +115,11 @@ options for details.\n \tvariable if it exists, or lexicographic order otherwise. See\n \tlinkgit:git-config[1].\n \n+--color[=<when>]:\n+\tRespect any colors specified in the `--format` option. The\n+\t`<when>` field must be one of `always`, `never`, or `auto` (if\n+\t`<when>` is absent, behave as if `always` was given).\n+\n -i::\n --ignore-case::\n \tSorting and filtering tags are case insensitive.\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 5d7c921a77..e931be9ce4 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -36,6 +36,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_INTEGER( 0 , \"count\", &maxcount, N_(\"show only <n> matched refs\")),\n \t\tOPT_STRING(  0 , \"format\", &format.format, N_(\"format\"), N_(\"format to use for the output\")),\n+\t\tOPT__COLOR(&format.use_color, N_(\"respect format colors\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n \t\t\t    N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n \t\tOPT_CALLBACK(0, \"points-at\", &filter.points_at,\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex c627794181..12dbbc56d9 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -411,6 +411,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t},\n \t\tOPT_STRING(  0 , \"format\", &format.format, N_(\"format\"),\n \t\t\t   N_(\"format to use for the output\")),\n+\t\tOPT__COLOR(&format.use_color, N_(\"respect format colors\")),\n \t\tOPT_BOOL('i', \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_END()\n \t};\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex d6bffe6273..6358134805 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -435,8 +435,8 @@ test_expect_success '%(color) does not show color without tty' '\n \ttest_cmp expected.bare actual\n '\n \n-test_expect_success 'color.ui=always can override tty check' '\n-\tgit -c color.ui=always for-each-ref --format=\"$color_format\" >actual.raw &&\n+test_expect_success '--color can override tty check' '\n+\tgit for-each-ref --color --format=\"$color_format\" >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expected.color actual\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0a86f8ea39..4e62c505fc 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1912,8 +1912,8 @@ test_expect_success TTY '%(color) present with tty' '\n \ttest_cmp expect.color actual\n '\n \n-test_expect_success 'color.ui=always overrides auto-color' '\n-\tgit -c color.ui=always tag $color_args >actual.raw &&\n+test_expect_success '--color overrides auto-color' '\n+\tgit tag --color $color_args >actual.raw &&\n \ttest_decode_color <actual.raw >actual &&\n \ttest_cmp expect.color actual\n '\n-- \n2.14.2.1079.gce6b466188\n\n"},{"id":"329571","messageId":"20171003134606.2r2653znqidt2mgx@sigill.intra.peff.net","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"[PATCH 12/12] color: make \"always\" the same as \"auto\" in config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-03T13:46:06Z","receivedAt":"2017-10-03T13:46:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be handy to use `--color=always` (or it's synonym\n`--color`) on the command-line to convince a command to\nproduce color even if it's stdout isn't going to the\nterminal or a pager.\n\nWhat's less clear is whether it makes sense to set config\nvariables like color.ui to `always`. For a one-shot like:\n\n  git -c color.ui=always ...\n\nit's potentially useful (especially if the command doesn't\ndirectly support the `--color` option). But setting `always`\nin your on-disk config is much muddier, as you may be\nsurprised when piped commands generate colors (and send them\nto whatever is consuming the pipe downstream).\n\nSome people have done this anyway, because:\n\n  1. The documentation for color.ui makes it sound like\n     using `always` is a good idea, when you almost\n     certainly want `auto`.\n\n  2. Traditionally not every command (and especially not\n     plumbing) respected color.ui in the first place. So\n     the confusion came up less frequently than it might\n     have.\n\nThe situation changed in 136c8c8b8f (color: check color.ui\nin git_default_config(), 2017-07-13), which negated point\n(2): now scripts using only plumbing commands (like\nadd-interactive) are broken by this setting.\n\nThat commit was fixing real issues (e.g., by making\n`color.ui=never` work, since `auto` is the default), so we\ndon't want to just revert it.  We could turn `always` into a\nnoop in plumbing commands, but that creates a hard-to-explain\ninconsistency between the plumbing and other commands.\n\nInstead, let's just turn `always` into `auto` for all config.\nThis does break the \"one-shot\" config shown above, but again,\nwe're probably better to have simple and consistent rules than\nto try to special-case command-line config.\n\nThere is one place where `always` should retain its meaning:\non the command line, `--color=always` should continue to be\nthe same as `--color`, overriding any isatty checks. Since the\ncommand-line parser also depends on git_config_colorbool(), we\ncan use the existence of the \"var\" string to deterine whether\nwe are serving the command-line or the config.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/config.txt   | 35 +++++++++++++++++------------------\n color.c                    |  2 +-\n t/t3701-add-interactive.sh | 10 ++++++++++\n 3 files changed, 28 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1ac0ae6adb..b53c994d0a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1058,10 +1058,10 @@ clean.requireForce::\n \n color.branch::\n \tA boolean to enable/disable color in the output of\n-\tlinkgit:git-branch[1]. May be set to `always`,\n-\t`false` (or `never`) or `auto` (or `true`), in which case colors are used\n-\tonly when the output is to a terminal. If unset, then the\n-\tvalue of `color.ui` is used (`auto` by default).\n+\tlinkgit:git-branch[1]. May be set to `false` (or `never`) to\n+\tdisable color entirely, `auto` (or `true` or `always`) in which\n+\tcase colors are used only when the output is to a terminal.  If\n+\tunset, then the value of `color.ui` is used (`auto` by default).\n \n color.branch.<slot>::\n \tUse customized color for branch coloration. `<slot>` is one of\n@@ -1072,12 +1072,11 @@ color.branch.<slot>::\n \n color.diff::\n \tWhether to use ANSI escape sequences to add color to patches.\n-\tIf this is set to `always`, linkgit:git-diff[1],\n+\tIf this is set to `true` or `auto`, linkgit:git-diff[1],\n \tlinkgit:git-log[1], and linkgit:git-show[1] will use color\n-\tfor all patches.  If it is set to `true` or `auto`, those\n-\tcommands will only use color when output is to the terminal.\n-\tIf unset, then the value of `color.ui` is used (`auto` by\n-\tdefault).\n+\twhen output is to the terminal. The value `always` is a\n+\thistorical synonym for `auto`.  If unset, then the value of\n+\t`color.ui` is used (`auto` by default).\n +\n This does not affect linkgit:git-format-patch[1] or the\n 'git-diff-{asterisk}' plumbing commands.  Can be overridden on the\n@@ -1141,12 +1140,12 @@ color.grep.<slot>::\n --\n \n color.interactive::\n-\tWhen set to `always`, always use colors for interactive prompts\n+\tWhen set to `true` or `auto`, use colors for interactive prompts\n \tand displays (such as those used by \"git-add --interactive\" and\n-\t\"git-clean --interactive\"). When false (or `never`), never.\n-\tWhen set to `true` or `auto`, use colors only when the output is\n-\tto the terminal. If unset, then the value of `color.ui` is\n-\tused (`auto` by default).\n+\t\"git-clean --interactive\") when the output is to the terminal.\n+\tWhen false (or `never`), never show colors. The value `always`\n+\tis a historical synonym for `auto`.  If unset, then the value of\n+\t`color.ui` is used (`auto` by default).\n \n color.interactive.<slot>::\n \tUse customized color for 'git add --interactive' and 'git clean\n@@ -1193,10 +1192,10 @@ color.ui::\n \tconfiguration to set a default for the `--color` option.  Set it\n \tto `false` or `never` if you prefer Git commands not to use\n \tcolor unless enabled explicitly with some other configuration\n-\tor the `--color` option. Set it to `always` if you want all\n-\toutput not intended for machine consumption to use color, to\n-\t`true` or `auto` (this is the default since Git 1.8.4) if you\n-\twant such output to use color when written to the terminal.\n+\tor the `--color` option. Set it to `true` or `auto` to enable\n+\tcolor when output is written to the terminal (this is also the\n+\tdefault since Git 1.8.4). The value `always` is a historical\n+\tsynonym for `auto`.\n \n column.ui::\n \tSpecify whether supported commands should output in columns.\ndiff --git a/color.c b/color.c\nindex 9ccd954d6b..9c0dc82370 100644\n--- a/color.c\n+++ b/color.c\n@@ -308,7 +308,7 @@ int git_config_colorbool(const char *var, const char *value)\n \t\tif (!strcasecmp(value, \"never\"))\n \t\t\treturn 0;\n \t\tif (!strcasecmp(value, \"always\"))\n-\t\t\treturn 1;\n+\t\t\treturn var ? GIT_COLOR_AUTO : 1;\n \t\tif (!strcasecmp(value, \"auto\"))\n \t\t\treturn GIT_COLOR_AUTO;\n \t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 39d0130f88..a49c12c79b 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -483,4 +483,14 @@ test_expect_success 'hunk-editing handles custom comment char' '\n \tgit diff --exit-code\n '\n \n+test_expect_success 'add -p works even with color.ui=always' '\n+\tgit reset --hard &&\n+\techo change >>file &&\n+\ttest_config color.ui always &&\n+\techo y | git add -p &&\n+\techo file >expect &&\n+\tgit diff --cached --name-only >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.14.2.1079.gce6b466188\n"},{"id":"329637","messageId":"xmqqr2ujfwia.fsf@gitster.mtv.corp.google.com","threadId":"46845","inReplyTo":"20171003133713.ccxv6clrmuuhhc3u@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-04T02:59:57Z","receivedAt":"2017-10-04T03:00:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Here's what I came up with on the \"color.ui=always is nonsense that we\n> should not offer\" front. The number of patches may be a little daunting,\n> but most of them are just removing cases of \"git -c color.ui=always\"\n> from the tests.\n>\n>   [01/12]: test-terminal: set TERM=vt100\n>   [02/12]: t4015: prefer --color to -c color.diff=always\n>   [03/12]: t4015: use --color with --color-moved\n>   [04/12]: t3701: use test-terminal to collect color output\n>   [05/12]: t7508: use test_terminal for color output\n>   [06/12]: t7502: use diff.noprefix for --verbose test\n>   [07/12]: t6006: drop \"always\" color config tests\n>   [08/12]: t3203: drop \"always\" color test\n>   [09/12]: t7301: use test_terminal to check color\n>   [10/12]: t3205: use --color instead of color.branch=always\n>   [11/12]: provide --color option for all ref-filter users\n>   [12/12]: color: make \"always\" the same as \"auto\" in config\n\nI'm shuffling these so that everything except 03/12 and 09/12 goes\non top of jk/ref-filter-colors (to be merged later for v2.14.x) to\ncreate jk/ui-color-always-to-auto-maint branch.\n\nAnother branch jk/ui-color-always-to-auto would fork from 'master'\nand have 03/12 and 09/12 (with a tweak to use vt100 explicitly, as\nwe lack 01/12 at that point) applied, and then merge the above one.\nAnd then queuing another one that drops \"env TERM=vt100\" tweak added\nto 09/12 would bring us to the same state as applying your 12 patches\ndirectly on top.  That will cook in 'next' down to 'master' to make\nsure we do not regress in 2.15.\n\nThanks.\n"},{"id":"329743","messageId":"20171005100615.khtsy4snduarin2i@sigill.intra.peff.net","threadId":"46845","inReplyTo":"xmqqr2ujfwia.fsf@gitster.mtv.corp.google.com","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-05T10:06:15Z","receivedAt":"2017-10-05T10:06:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 04, 2017 at 11:59:57AM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Here's what I came up with on the \"color.ui=always is nonsense that we\n> > should not offer\" front. The number of patches may be a little daunting,\n> > but most of them are just removing cases of \"git -c color.ui=always\"\n> > from the tests.\n> >\n> >   [01/12]: test-terminal: set TERM=vt100\n> >   [02/12]: t4015: prefer --color to -c color.diff=always\n> >   [03/12]: t4015: use --color with --color-moved\n> >   [04/12]: t3701: use test-terminal to collect color output\n> >   [05/12]: t7508: use test_terminal for color output\n> >   [06/12]: t7502: use diff.noprefix for --verbose test\n> >   [07/12]: t6006: drop \"always\" color config tests\n> >   [08/12]: t3203: drop \"always\" color test\n> >   [09/12]: t7301: use test_terminal to check color\n> >   [10/12]: t3205: use --color instead of color.branch=always\n> >   [11/12]: provide --color option for all ref-filter users\n> >   [12/12]: color: make \"always\" the same as \"auto\" in config\n> \n> I'm shuffling these so that everything except 03/12 and 09/12 goes\n> on top of jk/ref-filter-colors (to be merged later for v2.14.x) to\n> create jk/ui-color-always-to-auto-maint branch.\n\nThanks, I was so busy slogging through the set of broken tests that I\ndidn't even think that some of the cases would not be available on\nmaint.  Sorry to create work for you in untangling it.\n\n> Another branch jk/ui-color-always-to-auto would fork from 'master'\n> and have 03/12 and 09/12 (with a tweak to use vt100 explicitly, as\n> we lack 01/12 at that point) applied, and then merge the above one.\n> And then queuing another one that drops \"env TERM=vt100\" tweak added\n> to 09/12 would bring us to the same state as applying your 12 patches\n> directly on top.  That will cook in 'next' down to 'master' to make\n> sure we do not regress in 2.15.\n\nI eyeballed what you pushed out, and it all looks good to me (looks like\nyou did that final tweak as part of the merge of the two branches).\n\n-Peff\n"},{"id":"330733","messageId":"1508506274.23715.11.camel@gmail.com","threadId":"46845","inReplyTo":"20171003091049.jb46pyi3obtwkzsa@sigill.intra.peff.net","subject":"Re: Updated to v2.14.2 on macOS; git add --patch broken","fromName":"Jan Prachař","fromEmail":"jan.prachar@gmail.com","sentAt":"2017-10-20T13:31:14Z","receivedAt":"2017-10-20T13:31:23Z","isPatch":false,"sender":{"key":"jan.prachar@gmail.com","avatar":null},"body":"On Tue, 2017-10-03 at 05:10 -0400, Jeff King wrote:\n> I dunno. Maybe I am wrong, because certainly I never set it. We've\n> had\n> two reports on the list since v2.14.2. The motivation for the first\n> was\n> \"I have no idea why I set that, and I'll switch to auto\". This is the\n> second.\n> \n\nI also came across this git add -p regression. The reason I added\ncolor.ui =\nalways was that `git log --pretty=\"tformat:%Cgreen%h%Creset\" | less`\nstopped\nhaving colored output with color.ui = auto. And git config man page\nsuggeseted\ncolor.ui = always on the first place.\n"}]}