[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r15282 - in trunk/subversion: clients/cmdline

From: Joshua Varner <jlvarner_at_gmail.com>
Date: 2005-07-07 17:36:07 CEST

On 07 Jul 2005 08:43:16 -0500, kfogel@collab.net <kfogel@collab.net> wrote:
> Joshua Varner <jlvarner@gmail.com> writes:
> > On 7/6/05, kfogel@tigris.org <kfogel@tigris.org> wrote:
> > > Author: kfogel
> > > Date: Wed Jul 6 23:16:13 2005
> > > New Revision: 15282
> > >
> > <snip>
> > >
> > ==============================================================================
> > > --- trunk/subversion/clients/cmdline/util.c (original)
> > > +++ trunk/subversion/clients/cmdline/util.c Wed Jul 6 23:16:13 2005
> > > @@ -545,7 +545,7 @@
> > > else /* non_interactive flag says we can't pop up an editor, so error */
> > > {
> > > return svn_error_create
> > > _("Cannot invoke editor to get log message "
> > > "when non-interactive"));
> > > }
> > >
> > I may be missing something, but why isn't this check occuring at line 478:
> > while (! message)
> > {
> > /* We still don't have a valid commit message. Use $EDITOR to
> > get one. Note that svn_cl__edit_externally will still return
> > a UTF-8'ized log message. */
> >
> > This whole loop is exists to invoke the editor, so why enter it at all
> > if in interactive mode?
> > Then if that's the case shouldn't the error be either
> > specifying non-interactive without specifying a commit message on the
> > command line is the error. Or maybe a new error
> Thanks for reviewing the commit!
> Regarding the placement of the check:
> Well, I'm not sure that one way is better than the other. I
> considered placing it at the top of the loop, but felt that then the
> check wouldn't be localized enough. After all, the sin we're trying
> to avoid is that of actually starting up the editor, not that of doing
> the prep work for starting the editor -- prep work that might,
> someday, become applicable to something else in addition to starting
> an editor, at which point my overeager conditional would become a bug
> source.
> I don't feel strongly that one way is right and the other wrong
> though; if there's a good argument for placing the check earlier, we
> should do that. However I think that the argument "This way we'd
> avoid doing the prep work" would not be good enough, since we're going
> to return error for the first iteration anyway (I'm not sure you were
> actually making that argument, I'm just anticipating it).

Based on the comment at the top of that loop, I assumed it would only
apply to using the editor, and yes thought the prep work could be
avoided. But I see you point on that issue, I'm just not sure what
other method could use that prep work and still be non-interactive.

Given that argument the only other reason is personal style, to me it
would show the logical flow better to put the check before the loop,
and adding a new method of generating the comment would then either
fall naturally into the interactive or non-interactive part of the

> Regarding the error code:
> Oops, you're right. I didn't notice those codes when I was looking
> for an appropriate one to use. Actually there's an even more
> appropriate one: SVN_ERR_CL_INSUFFICIENT_ARGS. In r15285 I've gotten
> rid of the new error code and just used SVN_ERR_CL_INSUFFICIENT_ARGS.
> (I mean, lightblue.bikeshed.com and all that, but still you're right
> that we should avoid introducing new error codes if we can avoid it.)

I've never read that before, but agree 100%. With this particular
detail, I just believed from the viewpoint of a user with no knowledge
of the function, that they would not know what to do differently from
that error message. That is, they would have to follow this chain of

-Note my inner monologues are usually on the sarcastic end (not a
reflection on svn, more on my self)
  Write script
  Run it - damn didn't work
  Find error - Can't launch the editor when non-interactive
  Well, duh, I told you not to be interactive
  Wait why are you trying to launch the editor
  Oh, I'm an idiot I forgot the log message

I was just trying to skip the Well and Wait steps, because I have many
a day when that train of logic could take a lot longer than typing it,
especially in a large script with other possible sources of error.


To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 7 17:37:16 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.