{"thread":{"id":"15917","subject":"--diff-filter=T does not list x changes","startedAt":"2008-10-15T18:42:35Z","lastAt":"2008-10-19T10:29:59Z","messageCount":13,"participants":["Anders Melchiorsen","Jeff King","Junio C Hamano","Nanako Shiraishi"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"93114","messageId":"871vyhbsys.fsf@cup.kalibalik.dk","threadId":"15917","inReplyTo":null,"subject":"--diff-filter=T does not list x changes","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-10-15T18:42:35Z","receivedAt":"2008-10-15T18:42:35Z","isPatch":false,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":">From documentation, I would expect --diff-filter to list changes in\nthe execute bit, but it does not. I hear on #git that this is\nintended, though I still do not know how to filter on the execute bit.\nIs it impossible?\n\n\nTestcase:\n\n mkdir t && cd t && git init\n touch a && git add -A && git commit -m1\n chmod +x a && git add -A && git commit -m2\n git log --diff-filter=T # <--- shows nothing\n rm -f a && ln -s b a && git add -A && git commit -m3\n git log --diff-filter=T\n\n\nAnders\n"},{"id":"93181","messageId":"20081016102201.GB20762@sigill.intra.peff.net","threadId":"15917","inReplyTo":"871vyhbsys.fsf@cup.kalibalik.dk","subject":"Re: --diff-filter=T does not list x changes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-16T10:22:01Z","receivedAt":"2008-10-16T10:22:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 15, 2008 at 08:42:35PM +0200, Anders Melchiorsen wrote:\n\n> From documentation, I would expect --diff-filter to list changes in\n> the execute bit, but it does not. I hear on #git that this is\n> intended, though I still do not know how to filter on the execute bit.\n> Is it impossible?\n\nLooking at the code, I think it's impossible, and one would have to add\na new --diff-filter letter. However, at the very least, the\ndocumentation should clarify this situation. The --diff-filter\nexplanation says:\n\n Select only files [...] have their type (mode) changed (T) [...]\n\nwhich to me indicates that your test case should work. \n\n-Peff\n"},{"id":"93259","messageId":"7vhc7cq8uq.fsf@gitster.siamese.dyndns.org","threadId":"15917","inReplyTo":"20081016102201.GB20762@sigill.intra.peff.net","subject":"Re: --diff-filter=T does not list x changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-17T02:00:13Z","receivedAt":"2008-10-17T02:00:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Oct 15, 2008 at 08:42:35PM +0200, Anders Melchiorsen wrote:\n>\n>> From documentation, I would expect --diff-filter to list changes in\n>> the execute bit, but it does not. I hear on #git that this is\n>> intended, though I still do not know how to filter on the execute bit.\n>> Is it impossible?\n>\n> Looking at the code, I think it's impossible, and one would have to add\n> a new --diff-filter letter. However, at the very least, the\n> documentation should clarify this situation. The --diff-filter\n> explanation says:\n>\n> Select only files [...] have their type (mode) changed (T) [...]\n>\n> which to me indicates that your test case should work. \n\nThat documentation is quite loosely written. Typechange diff is what T\nhas always meant, and it never was about the executable bit. The word\n\"mode\" in that sentence only means the upper bits S_IFREG/S_IFLNK (iow,\nmasked by S_IFMT).\n"},{"id":"93269","messageId":"87ej2fvgv9.fsf@kalibalik.dk","threadId":"15917","inReplyTo":"7vhc7cq8uq.fsf@gitster.siamese.dyndns.org","subject":"Re: --diff-filter=T does not list x changes","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-10-17T07:08:10Z","receivedAt":"2008-10-17T07:08:10Z","isPatch":false,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> Select only files [...] have their type (mode) changed (T) [...]\n>>\n>> which to me indicates that your test case should work. \n>\n> That documentation is quite loosely written. Typechange diff is what\n> T has always meant, and it never was about the executable bit. The\n> word \"mode\" in that sentence only means the upper bits\n> S_IFREG/S_IFLNK (iow, masked by S_IFMT).\n\nI hope you agree that this reading is not obvious from the\ndocumentation, so I will send a patch later fixing up the prose (if\nnobody beats me to it).\n\nHow about adding a diff-filter=X for the executable bit? I could\nprobably look at that during the weekend.\n\n\nAnders.\n"},{"id":"93276","messageId":"7v1vyfoca2.fsf@gitster.siamese.dyndns.org","threadId":"15917","inReplyTo":"87ej2fvgv9.fsf@kalibalik.dk","subject":"Re: --diff-filter=T does not list x changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-17T08:29:09Z","receivedAt":"2008-10-17T08:29:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Melchiorsen writes:\n\n> Junio C Hamano writes:\n>\n>> That documentation is quite loosely written. Typechange diff is what\n>> T has always meant, and it never was about the executable bit. The\n>> word \"mode\" in that sentence only means the upper bits\n>> S_IFREG/S_IFLNK (iow, masked by S_IFMT).\n>\n> I hope you agree that this reading is not obvious from the\n> documentation,...\n\nYup, didn't I already say that the documentation is buggy?\n\n> How about adding a diff-filter=X for the executable bit?\n\nI do not think it is a good idea for two reasons. Backward compatibility\nand sane design.\n\nFor one thing, \"diff --name-status\" never shows X, so you would introduce\nan unnecessary inconsistency. If you change \"--name-status\" to avoid\nthat, you would be breaking people's existing scripts that expect to see\n\"M\" for such a change.\n\nEven if you were forgiven by these people whose scripts are broken by your\nchange, you need to decide between \"M\" and \"X\" when both contents and\nexecutable bit are changed. The least surprising logic would probably be\nto show \"X\" when _only_ executable bit is changed and show \"M\" when\ncontents changed (even when executable bit also did), but that feels quite\narbitrary. And the other way around isn't any better.\n"},{"id":"93308","messageId":"87wsg7m2xp.fsf@kalibalik.dk","threadId":"15917","inReplyTo":"7v1vyfoca2.fsf@gitster.siamese.dyndns.org","subject":"Re: --diff-filter=T does not list x changes","fromName":"Anders Melchiorsen","fromEmail":"anders@kalibalik.dk","sentAt":"2008-10-17T19:33:54Z","receivedAt":"2008-10-17T19:33:54Z","isPatch":false,"sender":{"key":"anders@kalibalik.dk","avatar":null},"body":"Junio C Hamano writes:\n\n> Anders Melchiorsen writes:\n>\n>> I hope you agree that this reading is not obvious from the\n>> documentation,...\n>\n> Yup, didn't I already say that the documentation is buggy?\n\nPossibly, though not in this thread.\n\n\n>> How about adding a diff-filter=X for the executable bit?\n>\n> I do not think it is a good idea for two reasons. Backward\n> compatibility and sane design.\n>\n> For one thing, \"diff --name-status\" never shows X, so you would\n> introduce an unnecessary inconsistency. If you change\n> \"--name-status\" to avoid that, you would be breaking people's\n> existing scripts that expect to see \"M\" for such a change.\n\n(I noticed that X is already used in diff-filter, but will keep it for\nthis discussion)\n\nI was thinking that X could be a subset of M. So only if you\nspecifically ask for diff-filter=X (and not M) would you get this new\nfunctionality. That should keep it compatible. It would then pick\nfiles that have had their x flipped, regardless of their change in\ncontent. With diff-filter=M, it would work as it does today.\n\nIf name-status output must be consistent, it could even output M for\nthese changes. That would still be unambiguous (but probably confusing).\n\n...\n\nAs you say that this is an unnecessary inconsistency, I wonder whether\nyou have a different way to pick out the commits that toggle the x\nbit? That is a problem that I am facing, with no solution shown so far ...\n\n\nAnders.\n"},{"id":"93324","messageId":"7vzll2epuh.fsf@gitster.siamese.dyndns.org","threadId":"15917","inReplyTo":"87wsg7m2xp.fsf@kalibalik.dk","subject":"Re: --diff-filter=T does not list x changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-17T23:58:30Z","receivedAt":"2008-10-17T23:58:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Melchiorsen writes:\n\n> ... way to pick out the commits that toggle the x\n> bit? That is a problem that I am facing, with no solution shown so far ...\n\nAre you interested in executable-bit only change, or any change that\ncontains changes to the executable-bit? If I were looking for the latter,\nprobably finding \"^:100664 100775 \" (or the other way around) in log --raw\n(or whatchanged) output would be what I would do --- the mode changes are\nrare enough in a sane project, so I wouldn't mind having to do such\nscripting as needed.\n\nThere are other \"commit pickers\" such as -S and --diff-filter\nthat do not absolutely have to exist (iow, they could also be scripted),\nbut what they pick earned easy shortcuts because the need is very common.\nOnce you can demonstrate that the need to pick executable-bit changes is\nalso very common, _and_ if you can come up with a clean solution, we might\nadd a commit picker that looks for changes in executable-ness in the\nfuture. I dunno.\n"},{"id":"93349","messageId":"87vdvq5lu4.fsf_-_@cup.kalibalik.dk","threadId":"15917","inReplyTo":"7vzll2epuh.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-10-18T08:49:55Z","receivedAt":"2008-10-18T08:49:55Z","isPatch":true,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"With the previous text, one could get the understanding that\ndiff-filter=T also tested for changes in the executable bit.\n\nSigned-off-by: Anders Melchiorsen \n---\n\nJunio C Hamano writes:\n\n> There are other \"commit pickers\" such as -S and\n> --diff-filter that do not absolutely have to exist (iow, they could\n> also be scripted), but what they pick earned easy shortcuts because\n> the need is very common. Once you can demonstrate that the need to\n> pick executable-bit changes is also very common, _and_ if you can\n> come up with a clean solution, we might add a commit picker that\n> looks for changes in executable-ness in the future. I dunno.\n\nYou are right that this should be a rare need.\n\nI didn't mean to push for this feature. I just offered to implement\nit, as I needed it myself and din't see other ways. A script will work\nfine for me, I should have thought of that.\n\nThe documentation patch that I promised is here.\n\n\nThanks,\nAnders.\n\n\n Documentation/diff-options.txt | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7788d4f..7604a13 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -137,7 +137,7 @@ endif::git-format-patch[]\n --diff-filter=[ACDMRTUXB*]::\n \tSelect only files that are Added (`A`), Copied (`C`),\n \tDeleted (`D`), Modified (`M`), Renamed (`R`), have their\n-\ttype (mode) changed (`T`), are Unmerged (`U`), are\n+\ttype (symlink/regular file) changed (`T`), are Unmerged (`U`), are\n \tUnknown (`X`), or have had their pairing Broken (`B`).\n \tAny combination of the filter characters may be used.\n \tWhen `*` (All-or-none) is added to the combination, all\n-- \n1.6.0.2.514.g23abd3\n"},{"id":"93367","messageId":"20081018224045.6117@nanako3.lavabit.com","threadId":"15917","inReplyTo":"87vdvq5lu4.fsf_-_@cup.kalibalik.dk","subject":"Re: [PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2008-10-18T13:40:45Z","receivedAt":"2008-10-18T13:40:45Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Anders Melchiorsen :\n\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 7788d4f..7604a13 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -137,7 +137,7 @@ endif::git-format-patch[]\n> --diff-filter=[ACDMRTUXB*]::\n> \tSelect only files that are Added (`A`), Copied (`C`),\n> \tDeleted (`D`), Modified (`M`), Renamed (`R`), have their\n> -\ttype (mode) changed (`T`), are Unmerged (`U`), are\n> +\ttype (symlink/regular file) changed (`T`), are Unmerged (`U`), are\n> \tUnknown (`X`), or have had their pairing Broken (`B`).\n> \tAny combination of the filter characters may be used.\n> \tWhen `*` (All-or-none) is added to the combination, all\n> -- \n> 1.6.0.2.514.g23abd3\n\nAre symlinks and regular files the only kind of object you can see in diff? What happens when a file or directory changes to a submodule?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"93381","messageId":"7vprlxbwtn.fsf@gitster.siamese.dyndns.org","threadId":"15917","inReplyTo":"87vdvq5lu4.fsf_-_@cup.kalibalik.dk","subject":"Re: [PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-18T18:08:20Z","receivedAt":"2008-10-18T18:08:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"},{"id":"93383","messageId":"7viqrpbvga.fsf@gitster.siamese.dyndns.org","threadId":"15917","inReplyTo":"20081018224045.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-18T18:37:57Z","receivedAt":"2008-10-18T18:37:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Quoting Anders Melchiorsen :\n>\n>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n>> index 7788d4f..7604a13 100644\n>> --- a/Documentation/diff-options.txt\n>> +++ b/Documentation/diff-options.txt\n>> @@ -137,7 +137,7 @@ endif::git-format-patch[]\n>> --diff-filter=[ACDMRTUXB*]::\n>> \tSelect only files that are Added (`A`), Copied (`C`),\n>> \tDeleted (`D`), Modified (`M`), Renamed (`R`), have their\n>> -\ttype (mode) changed (`T`), are Unmerged (`U`), are\n>> +\ttype (symlink/regular file) changed (`T`), are Unmerged (`U`), are\n>> \tUnknown (`X`), or have had their pairing Broken (`B`).\n>> \tAny combination of the filter characters may be used.\n>> \tWhen `*` (All-or-none) is added to the combination, all\n>> -- \n>> 1.6.0.2.514.g23abd3\n>\n> Are symlinks and regular files the only kind of object you can see in\n> diff? What happens when a file or directory changes to a submodule?\n\nOops. I've already applied Anders's patch, but you are right. A change\nfrom a blob to submodule also shows up as a typechange event.\n\nPerhaps we should just remove the parenthesised comment from there\ninstead. I'll rewind and rebuild, as I haven't pushed the results out\nyet (lucky me).\n"},{"id":"93405","messageId":"20081019100454.6117@nanako3.lavabit.com","threadId":"15917","inReplyTo":"87vdvq5lu4.fsf_-_@cup.kalibalik.dk","subject":"Re: [PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2008-10-19T01:04:54Z","receivedAt":"2008-10-19T01:04:54Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n>\n> Nanako Shiraishi writes:\n>\n>> Quoting Anders Melchiorsen :\n>>\n>>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n>>> index 7788d4f..7604a13 100644\n>>> --- a/Documentation/diff-options.txt\n>>> +++ b/Documentation/diff-options.txt\n>>> @@ -137,7 +137,7 @@ endif::git-format-patch[]\n>>> --diff-filter=[ACDMRTUXB*]::\n>>> \tSelect only files that are Added (`A`), Copied (`C`),\n>>> \tDeleted (`D`), Modified (`M`), Renamed (`R`), have their\n>>> -\ttype (mode) changed (`T`), are Unmerged (`U`), are\n>>> +\ttype (symlink/regular file) changed (`T`), are Unmerged (`U`), are\n>>> \tUnknown (`X`), or have had their pairing Broken (`B`).\n>>> \tAny combination of the filter characters may be used.\n>>> \tWhen `*` (All-or-none) is added to the combination, all\n>>> -- \n>>> 1.6.0.2.514.g23abd3\n>>\n>> Are symlinks and regular files the only kind of object you can see in\n>> diff? What happens when a file or directory changes to a submodule?\n>\n> Oops. I've already applied Anders's patch, but you are right. A change\n> from a blob to submodule also shows up as a typechange event.\n>\n> Perhaps we should just remove the parenthesised comment from there\n> instead. I'll rewind and rebuild, as I haven't pushed the results out\n> yet (lucky me).\n\nI see that you pushed out this change already, and you changed your mind and described them all. I think the result reads better.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"93437","messageId":"87bpxg513s.fsf@cup.kalibalik.dk","threadId":"15917","inReplyTo":"20081019100454.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Documentation: diff-filter=T only tests for symlink changes","fromName":"Anders Melchiorsen","fromEmail":"mail@cup.kalibalik.dk","sentAt":"2008-10-19T10:29:59Z","receivedAt":"2008-10-19T10:29:59Z","isPatch":true,"sender":{"key":"mail@cup.kalibalik.dk","avatar":null},"body":"Nanako Shiraishi writes:\n\n> I see that you pushed out this change already, and you changed your\n> mind and described them all. I think the result reads better.\n\nWhile we are fixing up that paragraph, this part could also use some\nelaboration:\n\n Unknown (`X`), or have had their pairing Broken (`B`).\n\nI would do it, but I have no idea what these two mean.\n\n\nRegards,\nAnders\n"}]}