{"thread":{"id":"9151","subject":"Re: diffutils: C labels misdetected as functions","startedAt":"2007-07-22T19:16:23Z","lastAt":"2007-07-22T21:30:00Z","messageCount":2,"participants":["Simon Arlott","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"48172","messageId":"46A3AD07.1070504@simon.arlott.org.uk","threadId":"9151","inReplyTo":"87abtogu6u.fsf@penguin.cs.ucla.edu","subject":"Re: diffutils: C labels misdetected as functions","fromName":"Simon Arlott","fromEmail":"simon@fire.lp0.eu","sentAt":"2007-07-22T19:16:23Z","receivedAt":"2007-07-22T19:16:23Z","isPatch":false,"sender":{"key":"simon@fire.lp0.eu","avatar":"https://avatars.githubusercontent.com/u/70171?v=4"},"body":"On 22/07/07 19:38, Paul Eggert wrote:\n> Simon Arlott <simon@fire.lp0.eu> writes:\n> \n>> The patch below stops diffutils treating C labels starting in column 1 as function names.\n> \n> That patch alone wouldn't suffice, since -p is documented as implying\n> -F '^[[:alpha:]$_]'.  If the behavior changes, we'd also have to\n> change the documentation to match.\n\nOk.\n\n> I'm not sure the change is a good idea.  It would complicate the\n> documentation and therefore the user interface, without that much\n\nComplicated documentation means a bug shouldn't be fixed?\n\n> benefit.  And it wouldn't suffice in general, since it would still\n> mishandle labels followed by comments or by white space.\n\nYes, the regexp needs changing further to ignore comments...\n\n\"[[:alpha:]$_][^:]*$\" perhaps? Although that ignores functions with\ncomments on the same line containing a :.\n\n> Also, for long C functions some people might rather see a top-level\n> (unindented) label highlighted than the function name highlighted, so\n> they might prefer the current behavior.\n\n\"-p  --show-c-function  Show which C function each change is in.\"\n\nIt's not \"--show-c-function-or-label\".\n\n> Anyway, to work around your problem without changing \"diff\", you can\n> use \"diff -u -F '^[[:alpha:]$_](|.*[^:])$'\" instead of \"diff -u -p\".\n\nPerhaps git could do this (or similar) as a workaround? (Cc:ed)\n\n> Or you can put a single space before the labels in question: they'll\n> still stand out plenty.\n\nThe code should definitely not need to be modified to work around a\ndiff bug.\n\n-- \nSimon Arlott\n"},{"id":"48195","messageId":"7vtzrwqg87.fsf@assigned-by-dhcp.cox.net","threadId":"9151","inReplyTo":"46A3AD07.1070504@simon.arlott.org.uk","subject":"Re: diffutils: C labels misdetected as functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-22T21:30:00Z","receivedAt":"2007-07-22T21:30:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Simon Arlott <simon@fire.lp0.eu> writes:\n\n> On 22/07/07 19:38, Paul Eggert wrote:\n>> \n>> Anyway, to work around your problem without changing \"diff\", you can\n>> use \"diff -u -F '^[[:alpha:]$_](|.*[^:])$'\" instead of \"diff -u -p\".\n>\n> Perhaps git could do this (or similar) as a workaround? (Cc:ed)\n>\n>> Or you can put a single space before the labels in question: they'll\n>> still stand out plenty.\n>\n> The code should definitely not need to be modified to work around a\n> diff bug.\n\nI think the word \"bug\" is just about documentation.  Promising\n\"show C function\" and including labels and/or not showing all\nfunctions would be a bug, but given that diff is about showing\ntextual differences between two text files with arbitrary kind\nof contents, I do not think it is reasonable to take that \"show\nC function\" promise too literally.\n\nTo avoid this, the description of the feature could be reworded\nto \"shows lines that match pattern '^[[:alpha:]$_]' on context\nlines, intent of the option being that the pattern tends to\nmatch the beginning of function in C language and help the\nreader to locate the context more easily, with a caveat that the\npattern matching is not meant to capture all functions nor only\nfunctions --- there will be false matches.\"\n\nBut that would be too long.  And as Paul showed, you can narrow\nor widen the set of lines that are shown with the -F option.  I\ndo not think this deserves to be called a \"bug\".\n\nYou can define your own customized \"function name\" pattern in\nyour .git/config (or $HOME/.gitconfig) file to be used by the\ngit-diff family with recent enough git via the attributes\nmechanism, but I do not think we should change the default,\nunless GNU diff changes its default.  Personally I'd rather not\nsee it change, though.\n"}]}