* Re: diffutils: C labels misdetected as functions
[not found] ` <87abtogu6u.fsf@penguin.cs.ucla.edu>
@ 2007-07-22 19:16 ` Simon Arlott
2007-07-22 21:30 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Simon Arlott @ 2007-07-22 19:16 UTC (permalink / raw
To: Paul Eggert
Cc: Matthew Wilcox, Denis Cheng, kernel-janitors, linux-kernel, git
On 22/07/07 19:38, Paul Eggert wrote:
> Simon Arlott <simon@fire.lp0.eu> writes:
>
>> The patch below stops diffutils treating C labels starting in column 1 as function names.
>
> That patch alone wouldn't suffice, since -p is documented as implying
> -F '^[[:alpha:]$_]'. If the behavior changes, we'd also have to
> change the documentation to match.
Ok.
> I'm not sure the change is a good idea. It would complicate the
> documentation and therefore the user interface, without that much
Complicated documentation means a bug shouldn't be fixed?
> benefit. And it wouldn't suffice in general, since it would still
> mishandle labels followed by comments or by white space.
Yes, the regexp needs changing further to ignore comments...
"[[:alpha:]$_][^:]*$" perhaps? Although that ignores functions with
comments on the same line containing a :.
> Also, for long C functions some people might rather see a top-level
> (unindented) label highlighted than the function name highlighted, so
> they might prefer the current behavior.
"-p --show-c-function Show which C function each change is in."
It's not "--show-c-function-or-label".
> Anyway, to work around your problem without changing "diff", you can
> use "diff -u -F '^[[:alpha:]$_](|.*[^:])$'" instead of "diff -u -p".
Perhaps git could do this (or similar) as a workaround? (Cc:ed)
> Or you can put a single space before the labels in question: they'll
> still stand out plenty.
The code should definitely not need to be modified to work around a
diff bug.
--
Simon Arlott
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: diffutils: C labels misdetected as functions
2007-07-22 19:16 ` diffutils: C labels misdetected as functions Simon Arlott
@ 2007-07-22 21:30 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2007-07-22 21:30 UTC (permalink / raw
To: Simon Arlott
Cc: Paul Eggert, Matthew Wilcox, Denis Cheng, kernel-janitors,
linux-kernel, git
Simon Arlott <simon@fire.lp0.eu> writes:
> On 22/07/07 19:38, Paul Eggert wrote:
>>
>> Anyway, to work around your problem without changing "diff", you can
>> use "diff -u -F '^[[:alpha:]$_](|.*[^:])$'" instead of "diff -u -p".
>
> Perhaps git could do this (or similar) as a workaround? (Cc:ed)
>
>> Or you can put a single space before the labels in question: they'll
>> still stand out plenty.
>
> The code should definitely not need to be modified to work around a
> diff bug.
I think the word "bug" is just about documentation. Promising
"show C function" and including labels and/or not showing all
functions would be a bug, but given that diff is about showing
textual differences between two text files with arbitrary kind
of contents, I do not think it is reasonable to take that "show
C function" promise too literally.
To avoid this, the description of the feature could be reworded
to "shows lines that match pattern '^[[:alpha:]$_]' on context
lines, intent of the option being that the pattern tends to
match the beginning of function in C language and help the
reader to locate the context more easily, with a caveat that the
pattern matching is not meant to capture all functions nor only
functions --- there will be false matches."
But that would be too long. And as Paul showed, you can narrow
or widen the set of lines that are shown with the -F option. I
do not think this deserves to be called a "bug".
You can define your own customized "function name" pattern in
your .git/config (or $HOME/.gitconfig) file to be used by the
git-diff family with recent enough git via the attributes
mechanism, but I do not think we should change the default,
unless GNU diff changes its default. Personally I'd rather not
see it change, though.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-07-22 21:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1184489571313-git-send-email-crquan@gmail.com>
[not found] ` <4699EEA9.6070709@simon.arlott.org.uk>
[not found] ` <20070720170743.GF14791@parisc-linux.org>
[not found] ` <46A0F282.8070103@simon.arlott.org.uk>
[not found] ` <20070720180019.GJ14791@parisc-linux.org>
[not found] ` <46A1A375.3020900@simon.arlott.org.uk>
[not found] ` <20070721135200.GQ14791@parisc-linux.org>
[not found] ` <46A25BE0.7000104@simon.arlott.org.uk>
[not found] ` <87abtogu6u.fsf@penguin.cs.ucla.edu>
2007-07-22 19:16 ` diffutils: C labels misdetected as functions Simon Arlott
2007-07-22 21:30 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).