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

Re: [PATCH] Make --accept match conflict handling prompt

From: Eric Gillespie <epg_at_google.com>
Date: 2007-10-01 21:38:20 CEST

Daniel Rall <dlr@collab.net> writes:

> I'm attaching some JavaHL changes which will need to go along with
> this patch.

Have you tested this? I'm reluctant to commit otherwise.

> > - const char *repos_file; /* repository's version of the file */
> > + const char *their_file; /* their version of the file */
> > + /* XXX I'd also prefer "my" here and "mine" below; why be inconsistent=
> ? */
>
> +1

Good, I'll finish the renaming then.

> > +svn_cl__conflict_baton_new(svn_cl__accept_t accept_which,
> > + apr_hash_t *config,
> > + const char *editor_cmd,
> > + apr_pool_t *pool);
>
> Subversion uses the verbs "create" or "make" more often than the word
> "new" in constructor function names.

Ah, OK. I'll look around to see which is more common.

> > +/* Return svn_cl__accept_t value corresponding to @ word.
> > + * @since New in 1.5.
> > + */
> > +svn_cl__accept_t
> > +svn_cl__accept_from_word(const char *word);
>
> While I see value in having the "since what version" info in the doc
> strings, these particular doc strings don't need to use Doxygen
> markup.

Hm, do we actively discourage it? It seems like a good habit to
stay in, and harmless enough.

> > /* A mindless implementation of svn_wc_conflict_resolver_func_t that
> > * does absolutely nothing to resolve conflicts. */
> > svn_error_t *
> > @@ -260,7 +311,7 @@
> > one of the 3 fulltexts, edit the merged file on the spot, or just
> > skip the conflict (to be resolved later). */
>
> =2E..and this (new) one is missing such markup.

Are you referring to svn_cl__conflict_handler? I didn't add it,
so I didn't add the markup. Will fix.

> > svn_error_t *
> > -svn_cl__interactive_conflict_handler(svn_wc_conflict_result_t *result,
> > +svn_cl__conflict_handler(svn_wc_conflict_result_t *result,
> > const svn_wc_conflict_description_t=
> *desc,
> > void *baton,
> > apr_pool_t *pool);
>
> Level of indentation is off due to (good) function rename.

Will fix.

> > --- subversion/svn/conflict-callbacks.c (revision 26821)
> > +++ subversion/svn/conflict-callbacks.c (working copy)
> =2E..
> > +svn_cl__accept_t
> > +svn_cl__accept_from_word(const char *word)
> > +{
> > + if (strcmp(word, "postpone") =3D=3D 0)
> > + return svn_cl__accept_postpone;
> > + if (strcmp(word, "base") =3D=3D 0)
> > + return svn_cl__accept_base;
> > + if (strcmp(word, "mine") =3D=3D 0)
> > + return svn_cl__accept_mine;
> > + if (strcmp(word, "theirs") =3D=3D 0)
> > + return svn_cl__accept_theirs;
> > + if (strcmp(word, "edit") =3D=3D 0)
> > + return svn_cl__accept_edit;
> > + if (strcmp(word, "launch") =3D=3D 0)
> > + return svn_cl__accept_launch;
> > + /* word is an invalid action. */
> > + return svn_cl__accept_invalid;
> > +}
>
> These magic strings are also used in the command-line help. Probably
> want to share them via #define's.

Right, will fix.

> > /* Utility to print a full description of the conflict. */
> > static svn_error_t *
> > print_conflict_description(const svn_wc_conflict_description_t *desc,
> > @@ -104,9 +138,9 @@
> > if (desc->base_file)
> > SVN_ERR(svn_cmdline_printf(pool, _(" Ancestor file: %s\n"),
> > desc->base_file));
> > - if (desc->repos_file)
> > - SVN_ERR(svn_cmdline_printf(pool, _(" Repository's file: %s\n"),
> > - desc->repos_file));
> > + if (desc->their_file)
> > + SVN_ERR(svn_cmdline_printf(pool, _(" Their file: %s\n"),
> > + desc->their_file));
> > if (desc->user_file)
> > SVN_ERR(svn_cmdline_printf(pool, _(" User's file: %s\n"),
> > desc->user_file));
>
> Here's another spot where you might want "my".

Will fix.

> > +svn_cl__conflict_handler(svn_wc_conflict_result_t *result,
> > + const svn_wc_conflict_description_t *desc,
> > + void *baton,
> > + apr_pool_t *pool)
> > {
>
> The changes to this routine are probably going to hork Augie F's patch.

Oh, somehow I overlooked that. I'll take a look at it; let's see
if we can get it committed first.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Oct 1 21:38:36 2007

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