You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gb...@apache.org on 2013/07/08 21:03:37 UTC
svn commit: r1500884 -
/subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
Author: gbg
Date: Mon Jul 8 19:03:37 2013
New Revision: 1500884
URL: http://svn.apache.org/r1500884
Log:
On the --invoke-diff-cmd branch: Fix help string and comment.
* subversion/svnlook/svnlook.c
(options_table): Reverse the order and adjuste help strings for
--invoke-diff-cmd and --diff_cmd, deprecating the latter.
(main): Fix comment.
Modified:
subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
Modified: subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c?rev=1500884&r1=1500883&r2=1500884&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c Mon Jul 8 19:03:37 2013
@@ -133,11 +133,11 @@ static const apr_getopt_option_t options
{"no-diff-deleted", svnlook__no_diff_deleted, 0,
N_("do not print differences for deleted files")},
- {"diff-cmd", svnlook__diff_cmd, 1,
- N_("use ARG as diff command")},
-
{"invoke-diff-cmd", svnlook__invoke_diff_cmd, 1,
- N_("use ARG as diff command (see svn help diff for details)")},
+ N_("Customizable diff command (see svn help diff)")},
+
+ {"diff-cmd", svnlook__diff_cmd, 1,
+ N_("deprecated, use --invoke-diff-cmd instead")},
{"ignore-properties", svnlook__ignore_properties, 0,
N_("ignore properties during the operation")},
@@ -2645,7 +2645,7 @@ main(int argc, const char *argv[])
_("Cannot use the '--show-inherited-props' option with the "
"'--revprop' option")));
- /* The --show-inherited-props and --revprop options may not co-exist. */
+ /* The --diff-cmd and --invoke-diff-cmd options may not co-exist. */
if (opt_state.diff_cmd && opt_state.invoke_diff_cmd)
SVN_INT_ERR(svn_error_create
(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
Re: svn commit: r1500884 -
/subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnl
ook.c
Posted by Daniel Shahaf <da...@elego.de>.
Johan Corveleyn wrote on Mon, Jul 08, 2013 at 23:23:58 +0200:
> On Mon, Jul 8, 2013 at 10:20 PM, Gabriela Gibson
> <ga...@gmail.com> wrote:
> > On Mon, Jul 8, 2013 at 8:23 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> >> Does --invoke-diff-cmd support the "simple invocation style of
> >> --diff-cmd" as well? I.e. can I do 'svnlook diff -r3 $REPOS
> >> --invoke-diff-cmd /usr/bin/diff' and get the same result as with
> >> --diff-cmd?
> >>
> > No, -- invoke-diff-cmd just takes whatever you give it, substitutes ;f1 ...
> > ;l3 and passes that expansion on to your diff command.
> >
> > I changed the help string in svnlook.c with the idea that it will move
> > people towards using invoke-diff-cmd (which does not have diff-cmd's
> > restrictions)
>
> I don't think changing svnlook's help text will influence how people
> use 'svn'.
I'll go futher: I don't think changing svn's help text influences how
people use 'svn'. For example, '-N' has been deprecated for years and
I still use it occasionally --- because it's less typing than
--depth=files.
> The audiences tend to be quite different (svnlook's users
> are mainly svn admins, which might be 0.1% of all svn users). I think
> it's more important that svnlook and svn are consistent (also in their
> help texts).
>
> >, but as you rightly point out, one man's restrictions are
> > another man's labour saving device :-)
> >
> > So I don't know what to do here, I can just change the help string back and
> > we keep diff-cmd around(and it keeps life simple for everyone) or, I can
> > made all the other help strings match the deprecation notice.
>
> I think 'diff-cmd' should stick around un-deprecated. I think people
> would see it as a regression if they can't just run 'svn diff
> --diff-cmd /usr/bin/diff' anymore but have to run 'svn diff
> --invoke-diff-cmd <provide all the right arguments myself>'. That's a
> lot more typing :-).
>
> Perhaps you could say that --diff-cmd ARG is now a shorthand for
> --invoke-diff-cmd ARG $somethingsomething, to correlate the two, but
> I'm not sure. It might be a way to demonstrate the use of
> --invoke-diff-cmd though, as an example.
My usual use-case for --diff-cmd is 'svn diff --diff-cmd =diff -x -pU5'
(that invokes /usr/bin/diff -pU5).
Re: svn commit: r1500884 - /subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Jul 8, 2013 at 10:20 PM, Gabriela Gibson
<ga...@gmail.com> wrote:
> On Mon, Jul 8, 2013 at 8:23 PM, Johan Corveleyn <jc...@gmail.com> wrote:
>>
>> On Mon, Jul 8, 2013 at 9:03 PM, <gb...@apache.org> wrote:
>> > Author: gbg
>>
>> I didn't realize you intended to deprecate --diff-cmd in favor of
>> --invoke-diff-cmd. Are you sure? I can see that they're related and
>> mutually exclusive, but not sure if they are direct "successors".
>>
> I think it depends on the perspective. Technically, diff-cmd is a subset of
> invoke-diff-cmd, but in practice, people will probably think of
> invoke-diff-cmd as an extension.
>
>>
>> You made this change only for 'svnlook diff', but not for 'svn diff'.
>> Is that intentional, or just Work In Progress?
>>
> diff-cmd itself has been rerouted through the code for invoke-diff-cmd and
> the code it used to run on has been deprecated in r1500647.
>
>> Does --invoke-diff-cmd support the "simple invocation style of
>> --diff-cmd" as well? I.e. can I do 'svnlook diff -r3 $REPOS
>> --invoke-diff-cmd /usr/bin/diff' and get the same result as with
>> --diff-cmd?
>>
> No, -- invoke-diff-cmd just takes whatever you give it, substitutes ;f1 ...
> ;l3 and passes that expansion on to your diff command.
>
> I changed the help string in svnlook.c with the idea that it will move
> people towards using invoke-diff-cmd (which does not have diff-cmd's
> restrictions)
I don't think changing svnlook's help text will influence how people
use 'svn'. The audiences tend to be quite different (svnlook's users
are mainly svn admins, which might be 0.1% of all svn users). I think
it's more important that svnlook and svn are consistent (also in their
help texts).
>, but as you rightly point out, one man's restrictions are
> another man's labour saving device :-)
>
> So I don't know what to do here, I can just change the help string back and
> we keep diff-cmd around(and it keeps life simple for everyone) or, I can
> made all the other help strings match the deprecation notice.
I think 'diff-cmd' should stick around un-deprecated. I think people
would see it as a regression if they can't just run 'svn diff
--diff-cmd /usr/bin/diff' anymore but have to run 'svn diff
--invoke-diff-cmd <provide all the right arguments myself>'. That's a
lot more typing :-).
Perhaps you could say that --diff-cmd ARG is now a shorthand for
--invoke-diff-cmd ARG $somethingsomething, to correlate the two, but
I'm not sure. It might be a way to demonstrate the use of
--invoke-diff-cmd though, as an example.
--
Johan
Re: svn commit: r1500884 - /subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
Posted by Gabriela Gibson <ga...@gmail.com>.
On Mon, Jul 8, 2013 at 8:23 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> On Mon, Jul 8, 2013 at 9:03 PM, <gb...@apache.org> wrote:
> > Author: gbg
>
> I didn't realize you intended to deprecate --diff-cmd in favor of
> --invoke-diff-cmd. Are you sure? I can see that they're related and
> mutually exclusive, but not sure if they are direct "successors".
>
> I think it depends on the perspective. Technically, diff-cmd is a subset
of invoke-diff-cmd, but in practice, people will probably think of
invoke-diff-cmd as an extension.
> You made this change only for 'svnlook diff', but not for 'svn diff'.
> Is that intentional, or just Work In Progress?
>
> diff-cmd itself has been rerouted through the code for invoke-diff-cmd and
the code it used to run on has been deprecated in r1500647.
Does --invoke-diff-cmd support the "simple invocation style of
> --diff-cmd" as well? I.e. can I do 'svnlook diff -r3 $REPOS
> --invoke-diff-cmd /usr/bin/diff' and get the same result as with
> --diff-cmd?
>
> No, -- invoke-diff-cmd just takes whatever you give it, substitutes ;f1
... ;l3 and passes that expansion on to your diff command.
I changed the help string in svnlook.c with the idea that it will move
people towards using invoke-diff-cmd (which does not have diff-cmd's
restrictions), but as you rightly point out, one man's restrictions are
another man's labour saving device :-)
So I don't know what to do here, I can just change the help string back and
we keep diff-cmd around(and it keeps life simple for everyone) or, I can
made all the other help strings match the deprecation notice.
Gabriela
Re: svn commit: r1500884 - /subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Jul 8, 2013 at 9:03 PM, <gb...@apache.org> wrote:
> Author: gbg
> Date: Mon Jul 8 19:03:37 2013
> New Revision: 1500884
>
> URL: http://svn.apache.org/r1500884
> Log:
> On the --invoke-diff-cmd branch: Fix help string and comment.
>
> * subversion/svnlook/svnlook.c
>
> (options_table): Reverse the order and adjuste help strings for
> --invoke-diff-cmd and --diff_cmd, deprecating the latter.
>
> (main): Fix comment.
>
> Modified:
> subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
>
> Modified: subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
> URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c?rev=1500884&r1=1500883&r2=1500884&view=diff
> ==============================================================================
> --- subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c (original)
> +++ subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c Mon Jul 8 19:03:37 2013
> @@ -133,11 +133,11 @@ static const apr_getopt_option_t options
> {"no-diff-deleted", svnlook__no_diff_deleted, 0,
> N_("do not print differences for deleted files")},
>
> - {"diff-cmd", svnlook__diff_cmd, 1,
> - N_("use ARG as diff command")},
> -
> {"invoke-diff-cmd", svnlook__invoke_diff_cmd, 1,
> - N_("use ARG as diff command (see svn help diff for details)")},
> + N_("Customizable diff command (see svn help diff)")},
> +
> + {"diff-cmd", svnlook__diff_cmd, 1,
> + N_("deprecated, use --invoke-diff-cmd instead")},
I didn't realize you intended to deprecate --diff-cmd in favor of
--invoke-diff-cmd. Are you sure? I can see that they're related and
mutually exclusive, but not sure if they are direct "successors".
You made this change only for 'svnlook diff', but not for 'svn diff'.
Is that intentional, or just Work In Progress?
Does --invoke-diff-cmd support the "simple invocation style of
--diff-cmd" as well? I.e. can I do 'svnlook diff -r3 $REPOS
--invoke-diff-cmd /usr/bin/diff' and get the same result as with
--diff-cmd?
--
Johan