Branko Čibej <brane@xbc.nu> writes:
> >Anybody have a chance to look over my latest patch? Haven't heard anything
> >back for several days.
>
> I remember one relevant comment that you haven't addressed yet (see
> attached message).
Well, let's not encourage poor Mark to keep putting time into this
unless it's really going to get merged in. Right now, we don't have a
consensus that the feature is even desirable, let alone desirable for
1.0.
I hate to say it, but the more I think about it, the less I'd like to
see it merged in. The possibility for unexpected consequences seems
awfully high. I don't mean bugs in the code, necessarily, just
unexpectednesses in the user's experience.
I certainly wouldn't veto it, if people feel strongly that it should
go in. But let's please settle *that* question before continuing the
patch review cycle, which could waste Mark's time (and annoy him,
though he's been awfully forgiving so far).
-K
> Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
> From: cmpilato@collab.net
> Subject: Re: [PATCH] Issue #1295 (take 2, with log)
> To: Mark Grosberg <mark@nolab.conman.org>
> Cc: Branko Čibej <brane@xbc.nu>, dev@subversion.tigris.org
> Date: 09 May 2003 02:58:12 -0500
> Reply-To: cmpilato@collab.net
>
> Mark Grosberg <mark@nolab.conman.org> writes:
>
> > I'd really rather not touch Python. I know Perl, but not Python and this
> > would take me a considerable amount of time to learn.
> >
> > > Modifying the commit list in a message passed in with -F should work, right?
> >
> > I don't know. The code is pretty hard to follow. Not even sure how -F
> > works.
>
> Your code should already do this correctly. However, I have another
> suggestion:
>
> > @@ -542,8 +671,16 @@
> >
> > /* Strip the prefix from the buffer. */
> > if (message)
> > + {
> > + char *commited_list = apr_pstrdup (pool, msg2);
> > + apr_hash_t *seen = apr_hash_make(pool);
> > +
> > + process_comitted_list (seen, commited_list, pool);
> > + flag_unmarked (lmb, commit_items, seen, pool);
> > +
> > truncate_buffer_at_prefix (&message->len, message->data,
> > EDITOR_EOF_PREFIX);
> > + }
>
> Move your process_comitted_list functionality *into* the
> truncate_buffer_at_prefix (renaming that function as appropriate, like
> handle_commit_paths() or something). Then make the new function do:
>
> 1. Search for the EDITOR_EOF_PREFIX. If not found, return. Else,
> remember the buffer location to "crop" to.
> 2. Handle the path list manipulations.
> 3. Crop (truncate) the buffer.
>
> No sense in pouring over this buffer twice.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
> ----------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 13 22:49:11 2003