On Thu, 16 Feb 2006, Julian Foad wrote:
> Peter N. Lundblad wrote:
> >
> > and the -p option of GNU diff (--show-c-function).
>
> I have to say I'm not at all keen on this, despite agreeing that it's
> useful to me and many of us in this community. I've written a whole
> essay of a reply, but I won't send that until I've slept on it.
>
Waiting for your rant:-) If you're objection is that this is somewhat
special-case-hackish, then I can agree. I would much rather like the more
general regexp support, but this would put a hard requirement on a regexp
library.
> > +/* Id for the --ignore-eol-style option, which doesn't have a short name. */
> > +#define SVN_DIFF__OPT_IGNORE_EOL_STYLE 256
> > +/* Options supported by svn_diff_file_options_parse(). */
> > +static const apr_getopt_option_t diff_options[] =
>
> Style nit: I'd prefer a blank line between one item that is commented on a
> separate line and the next. (Several such places.)
>
Good point. Fixed in some places that I found; hope I don't miss any.
> > +{
> > + { "ignore-space-change", 'b', 0, NULL },
> > + { "ignore-all-space", 'w', 0, NULL },
> > + { "ignore-eol-style", SVN_DIFF__OPT_IGNORE_EOL_STYLE, 0, NULL },
> > + { "show-c-function", 'p', 0, NULL },
> > + /* ### For compatibility; we don't support the argument to -u, because
> > + * ### we don't have optional argument support. */
> > + { "unified", 'u', 0, NULL },
>
> Current GNU diff doesn't accept an argument to "-u", but instead has a
> separate short option name "-U NUM", so I don't think you need that
> "###" comment.
I just tried diff -u1 a b and it worked, despite the documentation. And
--unified has the optional argument, so I think the comment is
appropriate.
Thanks for the review,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 16 08:26:53 2006