I've just come across this new function while reviewing the latest
"disambiguate (de)translation" patch.
I found this function very difficult to understand.
> /** Returns the @a eol and @a force_repair parameters needed to
> * translate @a style to its normal form (used for working copy text
> * bases and in the repository). @a eol is assumed to point to a
> * valid eol translation string and may be returned unmodified.
[...]
> svn_subst_eol_style_normalize (svn_boolean_t *force_repair,
> const char **eol,
> svn_subst_eol_style_t style);
The doc string seems to be written from the point of view of a user of some
other particular function(s). Please could you define this function in terms
unrelated to the other function(s). Also mention the "style" parameter. Also
the function's name doesn't clearly indicate what it does.
This function is used in two places: in svn_subst_translate_to_normal_form()
local to its definition, and in the new svn_wc_translated_file2() in libsvn_wc.
This function seems to return two bits of information that are only loosely
related. The "force_repair" decision is the province of libsvn_wc and
shouldn't be known at this level. It has little to do with determining how to
translate to Normal Form; it only controls whether the translation will fail if
the source has mixed EOLs.
Then, the whole concept of a "Normal Form" is a WC concept - well, and a
repository concept. It isn't really inherent to the mechanism of translating
EOL styles. Does anything apart from the WC need to convert to/from it? Maybe
other client-side repository accesses (like "svn cat URL"), so maybe knowledge
of Normal Form and functions to convert to/from it will have to stay in here
(libsvn_subr) just to share the knowledge between those callers.
RELATED FUNCTIONS
> /** Indicates whether the working copy and normalized versions of a file
> * with the given the parameters differ. If @a force_eol_check is true,
> * the routine also accounts for all translations required due to repairing
> * fixed eol styles.
> *
> * @since New in 1.4
> *
> */
> svn_boolean_t
> svn_subst_translation_required (svn_subst_eol_style_t style,
> const char *eol,
> apr_hash_t *keywords,
> svn_boolean_t special,
> svn_boolean_t force_eol_check);
> /** Convenience routine (wrapper around svn_subst_copy_and_translate3)
> * which detranslates the given @a src into @a dst. The parameters
> * specified should be those immediately taken from the files'
> * properties.
> *
> * @since New in 1.4
> *
> */
> svn_error_t *
> svn_subst_translate_to_normal_form (const char *src,
> const char *dst,
> svn_subst_eol_style_t eol_style,
> const char *eol_str,
> apr_hash_t *keywords,
> svn_boolean_t special,
> apr_pool_t *pool);
Perhaps some of the same arguments apply to these. At least the "disambiguate
(de)translation" patch should probably remove the word "detranslate" from this
doc string.
If we're having a "svn_subst_translate_to_normal_form", why not a
"svn_subst_translate_from_normal_form" to complement it?
> svn_error_t *
> svn_subst_eol_style_normalize (svn_boolean_t *force_repair,
> const char **eol,
> svn_subst_eol_style_t style)
> {
> *force_repair = style == svn_subst_eol_style_fixed;
> switch (style)
> {
> case svn_subst_eol_style_fixed:
> break;
>
> case svn_subst_eol_style_native:
> *eol = SVN_SUBST__DEFAULT_EOL_STR;
> break;
>
> case svn_subst_eol_style_none:
> *eol = NULL;
When the style is "none", isn't "*eol" already null?
> break;
>
> default:
> return svn_error_create (SVN_ERR_IO_UNKNOWN_EOL, NULL, NULL);
> }
>
> return SVN_NO_ERROR;
> }
- Julian
---------------------------------------------------------------------
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:57:32 2005