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