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

Re: [PATCH] Disambiguate (de)translation; followup.

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2005-11-19 00:23:37 CET

On Fri, Nov 18, 2005 at 03:42:10PM +0100, Erik Huelsmann wrote:
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 17435)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -64,23 +64,42 @@
> * @{
> */
>
> - /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> + /** Translate from Normal Format.
> + *
> + * The working copy text bases and repository files are stored
> + * in this form if they have any svn: properties set which
> + * require translation.

Actually, they're always stored in this format, aren't they?

Minor nit, since it's probably determinable by context, but: it's not
entirely clear what "this form" above refers to (also, nit: the 'form'
vs. 'format' thing I mentioned before). This is in the documentation
for a flag that translates _from_ Normal Format; it then says "are
stored in this form" which makes me slightly confused: is it referring
to the form I'm using this flag to translate to, or the 'Normal Format'
mentioned in the flag's summary?

> + *
> + * This flag excludes SVN_WC_TRANSLATE_FROM_NF

How about "cannot be used with" rather than "excludes"?

> + /** Prevent any repair of eol styles, erroring out if
> + * the input doesn't use one eol style consistently.
> + * All other appropriate translations will be performed regardless.
> + *
> + * This flag excludes the use of SVN_WC_FORCE_EOL_REPAIR.
> + */
> +#define SVN_WC_TRANSLATE_NO_EOL_REPAIR 0x00000002

> + /** Force repair of eol styles, making sure the output file contains
> + * one consistent eol style.
> + *
> + * This flag excludes the use of SVN_WC_NO_EOL_REPAIR.
> + */
> +#define SVN_WC_TRANSLATE_FORCE_EOL_REPAIR 0x00000004

Again, "cannot be used with" might be simpler. I'm confused about these
two options. From the descriptions, it seems like there must be some
default functionality to do with 'repairing eol styles' (which is what,
exactly? It's not been mentioned anywhere so far), one that's used when
neither flag is passed.

But I can't think what it would be. You've got 'do no repair', and
'force repair'. Is the default option 'do some repair'?

I assume that these options are only intended to be used with
_TRANSLATE_TO_NF? Probably worth documenting that, if so, in addition
to whatever repair we do by default.

>
> + /** Don't register a pool cleanup on the output file */
> +#define SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP 0x00000008
> +
> /** Guarantee a new file is created on successful return.
> * The default shortcuts translation by returning the path

nit: I keep reading 'default' as an adjective, and 'shortcuts' as a noun.
(The default shortcuts... what?). Perhaps 'default implementation
shortcuts...'.

> -/** Set @a *xlated_path to a path to translated copy of @src
> +/** Set @a *xlated_path to a path to a translated copy of @src
> * or to @a src itself if no translation is necessary.
> * That is, if @a versioned_file's properties indicate newline conversion or
> * keyword expansion, point @a *xlated_path to a copy of @a src
> * whose newlines and keywords are converted using the translation
> - * as requested by @a flags.
> + * as requested by @a flags. The default eol translation can be
> + * overridden by specifying either @c SVN_WC_TRANSLATE_NO_EOL_REPAIR or
> + * @c SVN_WC_TRANSLATE_FORCE_EOL_REPAIR.

Okay, so here's what those options do. I grok FORCE_EOL_REPAIR, but
not NO_EOL_REPAIR...

[delete three paragraphs where I try to guess what it does.]

No, sorry, I can't guess.

Also, you updated all the callers, but there don't appear to _be_ any
callers that pass these flags: do we really need them? Perhaps better
as a separate patch?

> - * Temporary files are created in the temp file area belonging to
> + * Output files are created in the temp file area belonging to
> * @a versioned_file.

Add something like 'By default, they will be deleted at pool cleanup
time.'?

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 19 00:24:29 2005

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