You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Justin Erenkrantz <je...@apache.org> on 2002/07/10 20:09:40 UTC
[PATCH] Allow revert --recursive to have implict dot-target
I know cmpilato isn't a fan of this, but I'm going to post this
for completeness-sake. If it goes in, yea. If not, it is here for
all to use even if cmpilato vetoes it. -- justin
Index: ./subversion/clients/cmdline/revert-cmd.c
===================================================================
--- ./subversion/clients/cmdline/revert-cmd.c
+++ ./subversion/clients/cmdline/revert-cmd.c 2002-07-10 12:58:17.000000000 -0700
@@ -48,9 +48,17 @@
SVN_ERR (svn_cl__args_to_target_array (&targets, os, opt_state,
FALSE, pool));
- /* Revert has no implicit dot-target `.', so don't you put that code here! */
if (! targets->nelts)
- return svn_error_create (SVN_ERR_CL_ARG_PARSING_ERROR, 0, 0, pool, "");
+ {
+ if (! recursive)
+ {
+ return svn_error_create (SVN_ERR_CL_ARG_PARSING_ERROR, 0, 0, pool,
+ "");
+ }
+
+ /* Do add "." if user passed 0 arguments and we are recursive. */
+ svn_cl__push_implicit_dot_target (targets, pool);
+ }
if (! opt_state->quiet)
svn_cl__get_notifier (¬ify_func, ¬ify_baton, FALSE, FALSE, pool);
Index: ./subversion/clients/cmdline/main.c
===================================================================
--- ./subversion/clients/cmdline/main.c
+++ ./subversion/clients/cmdline/main.c 2002-07-10 12:26:03.000000000 -0700
@@ -304,7 +304,10 @@
{ "revert", svn_cl__revert, {0},
"Restore pristine working copy file (undo all local edits)\n"
- "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n\n"
+ "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n"
+ " Unless --recursive is passed, revert does not have an implicit\n"
+ " target. Therefore, you must explicitly specify any targets\n"
+ " (such as `.').\n\n"
" Note: this routine does not require network access, and \n"
" resolves any conflicted states.\n",
{svn_cl__targets_opt, svn_cl__recursive_opt} },
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Justin Erenkrantz <je...@apache.org> writes:
> I know cmpilato isn't a fan of this, but I'm going to post this
> for completeness-sake. If it goes in, yea. If not, it is here for
> all to use even if cmpilato vetoes it. -- justin
Just asking: it passes "make check", right?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Martin Pool <mb...@samba.org>.
For what it's worth, I think it's a good tradeoff to require a
directory for "svn revert".
It rather reminds me of Linux killall vs SCO killall, which defaults
to killing all processes. ("Well, you did say so.")
It might be nice to actually not delete the reverted files, but rather
move them to something like "foo.~reverted~", so that there is some
ability to recover them if you want to.
--
Martin
<shawnm> why is the airport in Toronto called yyz?
<tpot_> xyzzy was too long
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Dave Cridland <da...@cridland.net>.
On Thu, 2002-07-11 at 13:34, Garrett Rooney wrote:
> On Thu, Jul 11, 2002 at 08:24:14AM -0400, mark benedetto king wrote:
>
> > I've noticed that Red Hat, and I presume other modern operating systems,
> > have started, aliasing rm to "rm -i". I suppose the argument is "if you're
> > not knowledgeable enough to unalias rm, you probably need the -i".
[snip]
> > This could be generalized to allow the user-agent to implement --force by
> > always returning "keep going" to these callbacks. If the API is sufficiently
> > rich, the user could choose, for example, "I mean what I say when I 'svn rm',
> > but prompt me for 'svn obliterate'."
I'm not sure these belong in the API, although it does mean that UIs
could share configuration for this. I'd be inclined to suggest a less
specific concept - classification of commands as "safe", "unsafe" and
"dangerous", with three confirmation levels. It's less configuration
data, and more useful if/when new commands are added.
And --force should force things which might break, and as such I'm not
sure the semantics fit svn at all. It would imply that a conflicting
file should be committed, which (I think) should never be allowed.
I think you want a -y/--yes - -f on rm is *not* the same as a lack of
-i, to continue your example, and I often wonder how many new RedHat
users are in the habit of using "rm -f" everywhere when they mean "rm"
without the "-i".
Otherwise I agree.
> if i wanted my computer to ask me 'do you mean it' every time i try to
> do something i'd be running a different operating system.
If I want, or don't want, my computer to ask me if I really mean to do
something, I'll turn it on, or off, respectively.
With a site-administrator hat on, I'd favourably consider defaulting
users to confirmation. I don't really mind how the source distribution
is shipped, personally, but then I can change the relevant parts to suit
my site.
> having a setting one can tweak to make this go away is NOT a valid
> solution. we should have reasonably safe/sane defaults and provide
> tweakable options only as a last resort.
Yes and no.
Yes - the defaults provided must be safe and sane.
No - options and configuration are a perfectly sensible way to ensure
that "irritating" safety can be turned off.
> big -1 on anything that makes the client ask 'are you sure?'. if you
> want that kind of behavior, write a gui client and give it all the
> little confirmation dialogs you want, but keep it out of my CLI.
You already have a confirmation of sorts on "svn revert" - you have to
explicitly type targets whereas usually the '.' is assumed. I'd prefer
it to be a simple question that I could --yes or configure away if I
felt like it.
I entirely understand that you wouldn't want an "Are you sure?"
confirmation - but I don't understand why you don't want anyone else to
have it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Thu, Jul 11, 2002 at 09:48:39AM -0400, mark benedetto king wrote:
> On Thu, Jul 11, 2002 at 08:34:51AM -0400, Garrett Rooney wrote:
> >
> > if i wanted my computer to ask me 'do you mean it' every time i try to
> > do something i'd be running a different operating system.
> >
>
> I know that you run FreeBSD, which probably means you have lots
> of scar tissue in your feet. :-)
*laugh*
> > having a setting one can tweak to make this go away is NOT a valid
> > solution. we should have reasonably safe/sane defaults and provide
> > tweakable options only as a last resort.
> >
>
> I'm okay with the default to be "destroy user data with wild abandon",
> as long as we explain at install time that new users may want to set
> the novice mode.
but we aren't defaulting to 'destroy user data with wild abandon',
we're doing the opposite. we're picking defaults that make it
intentionally difficult to accidentaly destroy user data.
> It is my experience that most systems do default to novice mode, and
> a one-time tweak for experienced users really isn't all that frustruating.
it irritates the hell out of me every time i have to check one of
those 'never show me this dialog again' check boxes, and this is the
same thing.
> > big -1 on anything that makes the client ask 'are you sure?'. if you
> > want that kind of behavior, write a gui client and give it all the
> > little confirmation dialogs you want, but keep it out of my CLI.
> >
>
> Well, the thing is, in order to put it in the GUI correctly, the system
> needs to be instrumented to actually call these callbacks.
i'm fine with having any callbacks that are needed, but in this case i
don't think they are.
> We can't expect the GUI designers to know that "svn foo" can occasionally
> be irreversible. Also, even if they *did* know that "svn foo" sometimes
> destroys user data, wouldn't it be nice if they didn't have to prompt
> *every* time, but only when it became an issue?
i don't think it's unreasonable to assume the programmer using our
libraries know these sort of things, as long as we document them.
> I think it would be neat if I could do "svn update --prompt-me"
> and it would say something like:
>
> U foo.c ? (Y/N) Y
> U bar.c ? (Y/N) N
> G foo.c ? (Y/N/E) E
>
>
> Where the "E" option for the merge would bring up $EDITOR with conflict
> markers in place, so that I can hand-merge. I used this sort of feature
> all the time with perforce (p4 resolve -v, IIRC). It rocked. Especially
> because I had files that I knew from experience wouldn't merge well (too
> much very similar code).
>
> You could also have --prompt=G,U. I don't know what the UI should be,
> I'm not a UI expert, but I do know that the functionality would be nice.
> Without a callback system this is really tricky work.
i agree there are situations where you need to ask the user for input
(like we do now for an empty log message from the editor), but i think
it's a subtle line between that and asking too many questions. as
long as we're picking sensible defaults, like requiring them to
specify a directory for a destructive command, then we shouldn't be
questioning what they tell us. the "should we default to . if revert
--recursive is done withtout a target" question should be solved by
looking at the use cases and making a decision weighing safety and
convenience, not by becoming overly chatty with the user.
it's a matter of degree. we should only ask a question if the user
does something really out of the ordinary (an empty log message), not
if they simply run a command (svn revert).
and while i have no problem with some kind of provision for callbacks
to help client writers out, i think 98% if these questions can be
solved at the client level, not at the subversion library level, just
like we do for the log message thing now.
-garrett
--
garrett rooney Remember, any design flaw you're
rooneg@electricjellyfish.net sufficiently snide about becomes
http://electricjellyfish.net/ a feature. -- Dan Sugalski
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Jul 11, 2002 at 08:34:51AM -0400, Garrett Rooney wrote:
>
> if i wanted my computer to ask me 'do you mean it' every time i try to
> do something i'd be running a different operating system.
>
I know that you run FreeBSD, which probably means you have lots
of scar tissue in your feet. :-)
> having a setting one can tweak to make this go away is NOT a valid
> solution. we should have reasonably safe/sane defaults and provide
> tweakable options only as a last resort.
>
I'm okay with the default to be "destroy user data with wild abandon",
as long as we explain at install time that new users may want to set
the novice mode.
It is my experience that most systems do default to novice mode, and
a one-time tweak for experienced users really isn't all that frustruating.
> big -1 on anything that makes the client ask 'are you sure?'. if you
> want that kind of behavior, write a gui client and give it all the
> little confirmation dialogs you want, but keep it out of my CLI.
>
Well, the thing is, in order to put it in the GUI correctly, the system
needs to be instrumented to actually call these callbacks.
We can't expect the GUI designers to know that "svn foo" can occasionally
be irreversible. Also, even if they *did* know that "svn foo" sometimes
destroys user data, wouldn't it be nice if they didn't have to prompt
*every* time, but only when it became an issue?
I think it would be neat if I could do "svn update --prompt-me"
and it would say something like:
U foo.c ? (Y/N) Y
U bar.c ? (Y/N) N
G foo.c ? (Y/N/E) E
Where the "E" option for the merge would bring up $EDITOR with conflict
markers in place, so that I can hand-merge. I used this sort of feature
all the time with perforce (p4 resolve -v, IIRC). It rocked. Especially
because I had files that I knew from experience wouldn't merge well (too
much very similar code).
You could also have --prompt=G,U. I don't know what the UI should be,
I'm not a UI expert, but I do know that the functionality would be nice.
Without a callback system this is really tricky work.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Jens Askengren <je...@linux.nu>.
tor 2002-07-11 klockan 14.34 skrev Garrett Rooney:
> big -1 on anything that makes the client ask 'are you sure?'. if you
> want that kind of behavior, write a gui client and give it all the
> little confirmation dialogs you want, but keep it out of my CLI.
Sometimes, when the program encounters a strang, errorlike situation it
might be usefull to ask the user.
For example, when you do "cvs commit" and omits the message, cvs asks if
you really want to commit without a message.
Another example: When you add a directory using TortoiseCVS and it finds
a file it doesent recoginize as text or binary, it will stop and ask if
you want to use -kb.
In these cases I personally think such "are you shure?" questions helps
to avoid problems, rather than just beeing anoying.
- Jens
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Thu, Jul 11, 2002 at 08:24:14AM -0400, mark benedetto king wrote:
> I've noticed that Red Hat, and I presume other modern operating systems,
> have started, aliasing rm to "rm -i". I suppose the argument is "if you're
> not knowledgeable enough to unalias rm, you probably need the -i".
>
> Frequently, one user interface for all users is not the most appropriate
> goal.
>
> What if, instead, svn config defaulted to "novice-mode", in which potentially
> destructive things called a user-agent-callback for confirmation. I'll bet
> GUI people would love to have that feature (to have the system tell them that
> something destructive and/or non-reversible was going to happen). In fact,
> the callback system may well be a pre-requisite for any real GUI work.
>
> This could be generalized to allow the user-agent to implement --force by
> always returning "keep going" to these callbacks. If the API is sufficiently
> rich, the user could choose, for example, "I mean what I say when I 'svn rm',
> but prompt me for 'svn obliterate'."
if i wanted my computer to ask me 'do you mean it' every time i try to
do something i'd be running a different operating system.
having a setting one can tweak to make this go away is NOT a valid
solution. we should have reasonably safe/sane defaults and provide
tweakable options only as a last resort.
big -1 on anything that makes the client ask 'are you sure?'. if you
want that kind of behavior, write a gui client and give it all the
little confirmation dialogs you want, but keep it out of my CLI.
-garrett
--
garrett rooney Remember, any design flaw you're
rooneg@electricjellyfish.net sufficiently snide about becomes
http://electricjellyfish.net/ a feature. -- Dan Sugalski
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by mark benedetto king <bk...@inquira.com>.
On Wed, Jul 10, 2002 at 06:30:28PM -0500, cmpilato@collab.net wrote:
> Alan Shutko <at...@acm.org> writes:
>
> > Justin Erenkrantz <je...@apache.org> writes:
> >
> > > cmpilato brought up that you could accidentally type 'svn revert
> > > --recursive' too, but I don't place much stock in that use case -
> > > there are easier ways to shoot yourself in the foot.
> >
> > What if you're about to type "svn revert --recursive subdir" but you
> > typo and hit return (or the cat walks on the keyboard, or whatever)
> > right before you type subdir?
>
> That's exactly the accident that has happened to me in the past. In
> fact, I made the mistake just this afternoon.
>
I've noticed that Red Hat, and I presume other modern operating systems,
have started, aliasing rm to "rm -i". I suppose the argument is "if you're
not knowledgeable enough to unalias rm, you probably need the -i".
Frequently, one user interface for all users is not the most appropriate
goal.
What if, instead, svn config defaulted to "novice-mode", in which potentially
destructive things called a user-agent-callback for confirmation. I'll bet
GUI people would love to have that feature (to have the system tell them that
something destructive and/or non-reversible was going to happen). In fact,
the callback system may well be a pre-requisite for any real GUI work.
This could be generalized to allow the user-agent to implement --force by
always returning "keep going" to these callbacks. If the API is sufficiently
rich, the user could choose, for example, "I mean what I say when I 'svn rm',
but prompt me for 'svn obliterate'."
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Mats Nilsson <ma...@xware.se>.
How about requiring --force whenever 'svn revert' would affect directories
and/or have recursive effects?
Mats
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by cm...@collab.net.
Alan Shutko <at...@acm.org> writes:
> Justin Erenkrantz <je...@apache.org> writes:
>
> > cmpilato brought up that you could accidentally type 'svn revert
> > --recursive' too, but I don't place much stock in that use case -
> > there are easier ways to shoot yourself in the foot.
>
> What if you're about to type "svn revert --recursive subdir" but you
> typo and hit return (or the cat walks on the keyboard, or whatever)
> right before you type subdir?
That's exactly the accident that has happened to me in the past. In
fact, I made the mistake just this afternoon.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Alan Shutko <at...@acm.org>.
Justin Erenkrantz <je...@apache.org> writes:
> cmpilato brought up that you could accidentally type 'svn revert
> --recursive' too, but I don't place much stock in that use case -
> there are easier ways to shoot yourself in the foot.
What if you're about to type "svn revert --recursive subdir" but you
typo and hit return (or the cat walks on the keyboard, or whatever)
right before you type subdir?
--
Alan Shutko <at...@acm.org> - In a variety of flavors!
The Founding Fathers warned us about this!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Justin Erenkrantz <je...@apache.org>.
On Wed, Jul 10, 2002 at 09:31:06PM +0100, Philip Martin wrote:
> Justin Erenkrantz <je...@apache.org> writes:
>
> > + "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n"
> > + " Unless --recursive is passed, revert does not have an implicit\n"
> > + " target. Therefore, you must explicitly specify any targets\n"
> > + " (such as `.').\n\n"
>
> We try to protect the user against "svn revert" errors but allow the
> more destructive "svn revert --recursive" to run unhindered. I don't
> understand the logic behind this behaviour.
The thought was that not having implicit targets was intended as a
dead-man's switch. But, with --recursive, my thought is the dead-man
switch has been pressed. If you type --recursive with no arguments,
I think you want '.'. cmpilato brought up that you could
accidentally type 'svn revert --recursive' too, but I don't place
much stock in that use case - there are easier ways to shoot
yourself in the foot.
On #svn, cmpilato made the following 3-step argument against this
(hopefully, he won't mind me quoting him):
1. it's fine for a subcommand to require an explicit target.
2. no existing subcommands have their target requirements change
by the presence/absence of a command-line option.
3. therefore, why introduce the inconsistency.
I just envision this getting used a lot esp. in httpd/APR since we
are always applying external patches to test. When we are done with
them, it'd be nice to just do a quick svn revert. IMHO, the current
interface of requiring the directory name is clunky.
I also don't think you can do anything very destructive since you
are using SVN as your version control software. If the change was
that critical, it should have been committed!
Regardless, this conversation has gone on too long for such a simple
patch... -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Ben Collins <bc...@debian.org>.
On Wed, Jul 10, 2002 at 09:31:06PM +0100, Philip Martin wrote:
> Justin Erenkrantz <je...@apache.org> writes:
>
> > + "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n"
> > + " Unless --recursive is passed, revert does not have an implicit\n"
> > + " target. Therefore, you must explicitly specify any targets\n"
> > + " (such as `.').\n\n"
>
> We try to protect the user against "svn revert" errors but allow the
> more destructive "svn revert --recursive" to run unhindered. I don't
> understand the logic behind this behaviour.
IMO, the more logical idea behind not having an implicit target with
"svn revert" is that it is not recursive by default. Which means an
implicit dot-target would simply revert the current wc directory. That's
usually not what the user wants.
However, if I take the time to do "svn revert --recursive", then It
should be reasonable to imply that I want to revert everything starting
at ".".
The original idea to protect the user is to not have implicit recursive.
That keeps bad things from happening. Having an implicit dot-target with
recursive enabled manually, is completely sane, and safe. The user
obviously knows what they want in that case.
Ben
--
Debian - http://www.debian.org/
Linux 1394 - http://linux1394.sourceforge.net/
Subversion - http://subversion.tigris.org/
Deqo - http://www.deqo.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Allow revert --recursive to have implict dot-target
Posted by Philip Martin <ph...@codematters.co.uk>.
Justin Erenkrantz <je...@apache.org> writes:
> + "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n"
> + " Unless --recursive is passed, revert does not have an implicit\n"
> + " target. Therefore, you must explicitly specify any targets\n"
> + " (such as `.').\n\n"
We try to protect the user against "svn revert" errors but allow the
more destructive "svn revert --recursive" to run unhindered. I don't
understand the logic behind this behaviour.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org