{"thread":{"id":"7638","subject":"[PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","startedAt":"2007-04-13T09:01:39Z","lastAt":"2007-04-13T14:26:30Z","messageCount":7,"participants":["Junio C Hamano","Johannes Sixt","Andy Parkins","Nicolas Pitre","Julian Phillips"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"39275","messageId":"7vodlsd4wc.fsf@assigned-by-dhcp.cox.net","threadId":"7638","inReplyTo":null,"subject":"[PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-13T09:01:39Z","receivedAt":"2007-04-13T09:01:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This makes paths with 'nodiff' attribute not to produce\n\"textual\" diffs from 'git-diff' family.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n attr.c |   18 ------------------\n diff.c |   40 +++++++++++++++++++++++++++++++---------\n 2 files changed, 31 insertions(+), 27 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex d35ae9e..bdbc4a3 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -299,21 +299,3 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)\n \t\trem = fill(path, pathlen, stk, check, num, rem);\n \treturn 0;\n }\n-\n-static void setup_binary_check(struct git_attr_check *check)\n-{\n-\tstatic struct git_attr *attr_binary;\n-\n-\tif (!attr_binary)\n-\t\tattr_binary = git_attr(\"binary\", 6);\n-\tcheck->attr = attr_binary;\n-}\n-\n-int git_path_is_binary(const char *path)\n-{\n-\tstruct git_attr_check attr_binary_check;\n-\n-\tsetup_binary_check(&attr_binary_check);\n-\treturn (!git_checkattr(path, 1, &attr_binary_check) &&\n-\t\t(0 < attr_binary_check.isset));\n-}\ndiff --git a/diff.c b/diff.c\nindex fbb79d7..bc4af5c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -8,6 +8,7 @@\n #include \"delta.h\"\n #include \"xdiff-interface.h\"\n #include \"color.h\"\n+#include \"attr.h\"\n \n #ifdef NO_FAST_WORKING_DIRECTORY\n #define FAST_WORKING_DIRECTORY 0\n@@ -1051,13 +1052,34 @@ static void emit_binary_diff(mmfile_t *one, mmfile_t *two)\n \temit_binary_diff_body(two, one);\n }\n \n+static void setup_nodiff_check(struct git_attr_check *check)\n+{\n+\tstatic struct git_attr *attr_nodiff;\n+\n+\tif (!attr_nodiff)\n+\t\tattr_nodiff = git_attr(\"nodiff\", 6);\n+\tcheck->attr = attr_nodiff;\n+}\n+\n #define FIRST_FEW_BYTES 8000\n-static int mmfile_is_binary(mmfile_t *mf)\n+static int file_is_binary(struct diff_filespec *one)\n {\n-\tlong sz = mf->size;\n+\tunsigned long sz;\n+\tstruct git_attr_check attr_nodiff_check;\n+\n+\tsetup_nodiff_check(&attr_nodiff_check);\n+\tif (!git_checkattr(one->path, 1, &attr_nodiff_check) &&\n+\t    (0 < attr_nodiff_check.isset))\n+\t\treturn 1;\n+\tif (!one->data) {\n+\t\tif (!DIFF_FILE_VALID(one))\n+\t\t\treturn 0;\n+\t\tdiff_populate_filespec(one, 0);\n+\t}\n+\tsz = one->size;\n \tif (FIRST_FEW_BYTES < sz)\n \t\tsz = FIRST_FEW_BYTES;\n-\treturn !!memchr(mf->ptr, 0, sz);\n+\treturn !!memchr(one->data, 0, sz);\n }\n \n static void builtin_diff(const char *name_a,\n@@ -1114,7 +1136,7 @@ static void builtin_diff(const char *name_a,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (!o->text && (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))) {\n+\tif (!o->text && (file_is_binary(one) || file_is_binary(two))) {\n \t\t/* Quite common confusing case */\n \t\tif (mf1.size == mf2.size &&\n \t\t    !memcmp(mf1.ptr, mf2.ptr, mf1.size))\n@@ -1190,7 +1212,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2)) {\n+\tif (file_is_binary(one) || file_is_binary(two)) {\n \t\tdata->is_binary = 1;\n \t\tdata->added = mf2.size;\n \t\tdata->deleted = mf1.size;\n@@ -1228,7 +1250,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n \n-\tif (mmfile_is_binary(&mf2))\n+\tif (file_is_binary(two))\n \t\treturn;\n \telse {\n \t\t/* Crazy xdl interfaces.. */\n@@ -1805,8 +1827,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \n \t\tif (o->binary) {\n \t\t\tmmfile_t mf;\n-\t\t\tif ((!fill_mmfile(&mf, one) && mmfile_is_binary(&mf)) ||\n-\t\t\t    (!fill_mmfile(&mf, two) && mmfile_is_binary(&mf)))\n+\t\t\tif ((!fill_mmfile(&mf, one) && file_is_binary(one)) ||\n+\t\t\t    (!fill_mmfile(&mf, two) && file_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n@@ -2701,7 +2723,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\t\treturn error(\"unable to read files to diff\");\n \n \t\t/* Maybe hash p->two? into the patch id? */\n-\t\tif (mmfile_is_binary(&mf2))\n+\t\tif (file_is_binary(p->two))\n \t\t\tcontinue;\n \n \t\tlen1 = remove_space(p->one->path, strlen(p->one->path));\n-- \n1.5.1.1.784.g95e2\n"},{"id":"39279","messageId":"461F602C.E9803108@eudaptics.com","threadId":"7638","inReplyTo":"7vodlsd4wc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-04-13T10:49:16Z","receivedAt":"2007-04-13T10:49:16Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano wrote:\n> \n> This makes paths with 'nodiff' attribute not to produce\n> \"textual\" diffs from 'git-diff' family.\n\nIf saying \"nodiff\" can be made equivalent to \"!diff\", then I'd strongly\nprefer an attribute \"diff\" over \"nodiff\". I'm a strong disbeliever in\ndouble negation.\n\n-- Hannes \"Can't we not have no double negation please?\" Sixt\n"},{"id":"39280","messageId":"200704131230.41594.andyparkins@gmail.com","threadId":"7638","inReplyTo":"461F602C.E9803108@eudaptics.com","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-13T11:30:38Z","receivedAt":"2007-04-13T11:30:38Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Friday 2007, April 13, Johannes Sixt wrote:\n\n> -- Hannes \"Can't we not have no double negation please?\" Sixt\n\nThere's nothing wrong with double negatives per se.  They often confer \nmore meaning than simple logic would suggest.\n\nFor example:\n\n I can't not hate CVS\n I hate CVS\n\nLogically identical, but semantically different.  In the first, the \nspeak would be suggesting that they've tried, but failed, to like CVS; \nin the second the speaker just hates it.  Language isn't logic, it's \nfuzzy logic :-).\n\nThe reason I think it's relevant to bring this up is that I think \nidentifier naming in programming should try to use language to lead the \nreader down the same thought path as the writer.\n\nYou want a flag that controls whether a thread is running - should it be \ncalled RunFlag or StopFlag?\n\n while( RunFlag )\n\n while( !StopFlag )\n\nI think that the context is important.\n\nI, personally, wouldn't like to say which is correct in that case - or \nin the \"nodiff\"/\"!diff\" question.  However, I don't think it's correct \nto universally rule out all double negative use - they have their \nplace.\n\n\n\nAndy\n\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39282","messageId":"461F6D74.63EF2769@eudaptics.com","threadId":"7638","inReplyTo":"200704131230.41594.andyparkins@gmail.com","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-04-13T11:45:56Z","receivedAt":"2007-04-13T11:45:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andy Parkins wrote:\n> I, personally, wouldn't like to say which is correct in that case - or\n> in the \"nodiff\"/\"!diff\" question.  However, I don't think it's correct\n> to universally rule out all double negative use - they have their\n> place.\n\nYet in Junio's introductory message, the example was\n\n        *       crlf !nodiff\n\nand it made me think: huh? And then again: huh?\n\nI don't mind if you call me dumb now, but even an intelligent person\nwill have to think a second or two about the meaning. That's plainly not\nnecessary. In particular, where there can be no difference between\n\"diff\" and \"!nodiff\".\n\n-- Hannes\n"},{"id":"39283","messageId":"200704131354.57056.andyparkins@gmail.com","threadId":"7638","inReplyTo":"461F6D74.63EF2769@eudaptics.com","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-13T12:54:55Z","receivedAt":"2007-04-13T12:54:55Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Friday 2007, April 13, Johannes Sixt wrote:\n\n> Yet in Junio's introductory message, the example was\n>\n>         *       crlf !nodiff\n>\n> and it made me think: huh? And then again: huh?\n>\n> I don't mind if you call me dumb now, but even an intelligent person\n\nWouldn't dream of it.\n\n> will have to think a second or two about the meaning. That's plainly\n> not necessary. In particular, where there can be no difference\n> between \"diff\" and \"!nodiff\".\n\nAs I said, I wouldn't like to make the call.\n\nI understand your point - however I think I understand why Junio chose \nnodiff as the attribute.  I would imagine that git /wants/ to show a \ntextual diff for everything as its default.  In that case, the \nattribute that you would assign to the least number of files \nis \"nodiff\".  Mostly you want diffs, so the attribute implemented is \nfor the special case and is \"nodiff\".\n\nIf I've understood the attributes system correctly, it's worth bearing \nin mind that \"!diff\" is not the same as \"nodiff\".  The not in \"!diff\" \nmeans \"supress the attribute diff\", not as you would traditionally \nimagine \"not-diff\".  In this case it doesn't matter because there are \nonly two possibilities for the diff engine - show the diff or don't.  \nHowever, more attributes might get added that aren't so simple.\n\nJunio - This makes me think that perhaps \"!\" is not the right symbol for \nthis - that's going to get read by programmers everywhere as NOT, \nrather than suppress.  Perhaps it doesn't matter, I might be \nover-thinking this.\n\n\n\nAndy\n\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39284","messageId":"alpine.LFD.0.98.0704131007140.28181@xanadu.home","threadId":"7638","inReplyTo":"200704131354.57056.andyparkins@gmail.com","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-13T14:09:37Z","receivedAt":"2007-04-13T14:09:37Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 13 Apr 2007, Andy Parkins wrote:\n\n> If I've understood the attributes system correctly, it's worth bearing \n> in mind that \"!diff\" is not the same as \"nodiff\".  The not in \"!diff\" \n> means \"supress the attribute diff\", not as you would traditionally \n> imagine \"not-diff\".  In this case it doesn't matter because there are \n> only two possibilities for the diff engine - show the diff or don't.  \n> However, more attributes might get added that aren't so simple.\n> \n> Junio - This makes me think that perhaps \"!\" is not the right symbol for \n> this - that's going to get read by programmers everywhere as NOT, \n> rather than suppress.  Perhaps it doesn't matter, I might be \n> over-thinking this.\n\nMaybe \"-\" then?  That better convey the notion of suppression while \"!\" \nis more about negation.\n\n\nNicolas\n"},{"id":"39285","messageId":"Pine.LNX.4.64.0704131525260.12853@reaper.quantumfyre.co.uk","threadId":"7638","inReplyTo":"alpine.LFD.0.98.0704131007140.28181@xanadu.home","subject":"Re: [PATCH 3/3] Teach 'diff' about 'nodiff' attribute.","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-13T14:26:30Z","receivedAt":"2007-04-13T14:26:30Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Fri, 13 Apr 2007, Nicolas Pitre wrote:\n\n> On Fri, 13 Apr 2007, Andy Parkins wrote:\n>\n>> If I've understood the attributes system correctly, it's worth bearing\n>> in mind that \"!diff\" is not the same as \"nodiff\".  The not in \"!diff\"\n>> means \"supress the attribute diff\", not as you would traditionally\n>> imagine \"not-diff\".  In this case it doesn't matter because there are\n>> only two possibilities for the diff engine - show the diff or don't.\n>> However, more attributes might get added that aren't so simple.\n>>\n>> Junio - This makes me think that perhaps \"!\" is not the right symbol for\n>> this - that's going to get read by programmers everywhere as NOT,\n>> rather than suppress.  Perhaps it doesn't matter, I might be\n>> over-thinking this.\n>\n> Maybe \"-\" then?  That better convey the notion of suppression while \"!\"\n> is more about negation.\n\n\"-flag\" will certainly be a more familiar way to say \"disable flag\" to \ngentoo users ... ;)\n\n-- \nJulian\n\n  ---\nLie, n.:\n \tA very poor substitute for the truth, but the only one\n \tdiscovered to date.\n"}]}