You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Malcolm Rowe <ma...@farside.org.uk> on 2005/11/15 20:17:44 UTC
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Couple of comments on the documentation changes (only), inline:
On Tue, Nov 15, 2005 at 01:36:57PM -0600, dionisos@tigris.org wrote:
> Author: dionisos
> Date: Tue Nov 15 13:36:57 2005
> New Revision: 17362
>
> * subversion/libsvn_wc/update_editor.c
> Update callers to svn_wc_translated_file and
> svn_wc_translated_file2, removing superseeded static functions.
"superseded"
> --- trunk/subversion/include/svn_wc.h (original)
> +++ trunk/subversion/include/svn_wc.h Tue Nov 15 13:36:57 2005
> + /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> +#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
> +
> + /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
> +#define SVN_WC_TRANSLATE_TO_NF 0x00000001
What is 'Normal Form(at)'? I guess (from later) that it's the format
that the text-base is stored in: LF line termination and unexpanded
keywords. But it'd be good to document it here, since we don't
use the phrase elsewhere. (Hmm, would something even clearer like
SVN_WC_TRANSLATE_TEXTBASE_TO_WORKING be better? Probably too long.)
> +
> + /** Only do translation associated with the svn:special property only */
> +#define SVN_WC_TRANSLATE_SPECIAL_ONLY 0x00000002
Offhand, I don't know what kind of translation we'd do with respect to the
svn:special property at all. Could we have some more information here?
> +
> + /** Translate the special property only */
> +#define SVN_WC_TRANSLATE_DEL_TMP_ON_POOL_CLEANUP 0x00000004
Comment is wrong.
> +/** Set @a *xlated_path to a path to translated copy of @src
> + * or to @a src itself if no translation is necessary.
"(unless SVN_WC_TRANSLATE_FORCE_COPY is set)"? I know, you mention it
further down, maybe it's unimportant.
> + * If @c SVN_WC_TRANSLATE_DEL_TEMP_ON_POOL_CLEANUP is specified,
> + * a pool cleanup handler is registered on *xlated_p if it points to a
> + * temporary file.
> *
> * If an error is returned, the effect on @a *xlated_p is undefined.
"xlated_path", both times.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
> > > > (Hmm, would something even clearer like
> > > > SVN_WC_TRANSLATE_TEXTBASE_TO_WORKING be better? Probably too long.)
> > >
> > > But: it's not only the text base, it's also the contents as stored in
> > > the repository.
> > >
>
> It might be better to call this "Repository Format" as that is more
> explicit, and
> will be less confusing in the long term as features like compressed text
> base are introduced, since at that point the text-bases will no longer be
> in "Normal Format".
No, but the contents of the zipped file will be in 'normal form'
(though not in any particular format). It's more of a system wide
thing than either specific to the repository or working copy that's
why I didn't name it repository form(at).
bye,
Erik.
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Joshua Varner <jl...@gmail.com>.
On 11/17/05, Malcolm Rowe <ma...@farside.org.uk> wrote:
> On Thu, Nov 17, 2005 at 11:16:15AM +0100, Erik Huelsmann wrote:
> > > > + /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> > > > +#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
> > > > +
> > > > + /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
> > > > +#define SVN_WC_TRANSLATE_TO_NF 0x00000001
> > >
> > > What is 'Normal Form(at)'?
> >
> > Well, the essence of my changes are actually to keep that an
> > implementation detail. The user of these constants should not have to
> > know what it is.
> >
>
> Julian got it right across-thread from here - I was asking for an
> explanation as to what 'Normal Form' meant, because I couldn't really tell
> from the context what it was, or when I'd want to use it. You're right
> that the exact implementation details shouldn't be included here.
>
[snip]
> As a user of this API, all I really needed to know is that: the format
> of a file stored in the repository or text-base is not the same as one
> stored in the working copy, for several reasons (of which line endings and
> keywords are two well-known ones). 'Normal Format' refers to the format
> of the file in the repository and text-base, and this function translates
> between 'Normal Format' and the format seen directly by the user.
>
> Is it Normal Form or Normal Format, by the way? You're using the former,
> but the doc-comments use the latter.
>
> And is it a phrase we normally use, or was it made up just for this
> function? I only ask because this is the first time I've ever run across
> it in the context of Subversion, and I'm wondering whether it's the best
> choice of words to describe the concept. (But then I'm a database guy
> by trade, so 'Normal Form' already means something completely different
> to me :-)).
>
> > > But it'd be good to document it here, since we don't
> > > use the phrase elsewhere.
> >
> > Then maybe the README.txt in libsvn_wc should be expanded, or some
> > document in notes/ should be added explaining what the normal form is
> > defined as. All a programmer should need to know (IMO) is that we
> > have such a thing and that you need to translate a WC file to it
> > before sending it to the repository.
> >
>
> To be clear: "it" in my sentence meant just an explanation of what
> the phrase meant, and why we need to care. A document in notes/ would
> probably be helpful too, but my main concern was for consumers of the API.
>
> > > (Hmm, would something even clearer like
> > > SVN_WC_TRANSLATE_TEXTBASE_TO_WORKING be better? Probably too long.)
> >
> > But: it's not only the text base, it's also the contents as stored in
> > the repository.
> >
It might be better to call this "Repository Format" as that is more
explicit, and
will be less confusing in the long term as features like compressed text base
are introduced, since at that point the text-bases will no longer be in "Normal
Format".
Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Thu, Nov 17, 2005 at 11:16:15AM +0100, Erik Huelsmann wrote:
> > > + /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> > > +#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
> > > +
> > > + /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
> > > +#define SVN_WC_TRANSLATE_TO_NF 0x00000001
> >
> > What is 'Normal Form(at)'?
>
> Well, the essence of my changes are actually to keep that an
> implementation detail. The user of these constants should not have to
> know what it is.
>
Julian got it right across-thread from here - I was asking for an
explanation as to what 'Normal Form' meant, because I couldn't really tell
from the context what it was, or when I'd want to use it. You're right
that the exact implementation details shouldn't be included here.
> > I guess (from later) that it's the format
> > that the text-base is stored in: LF line termination and unexpanded
> > keywords.
>
> Ah! See? This is exactly why it should be a (hidden) implementation
> fact: you have it only partly right. Yes, some files have LF
> termination. Others have CRLF termination as their normal form.
> Keywords are always stored unexpanded. But, the normal form for a
> symlink is entirely different from the actual versioned file (ie not a
> symlink).
>
I'd forgotten about symlinks, thanks (and the fact that it's also used
for the repository, which you mention below). But I probably shouldn't
have tried to include a (partial, as it turned out) technical definition,
since that's only confused the question.
As a user of this API, all I really needed to know is that: the format
of a file stored in the repository or text-base is not the same as one
stored in the working copy, for several reasons (of which line endings and
keywords are two well-known ones). 'Normal Format' refers to the format
of the file in the repository and text-base, and this function translates
between 'Normal Format' and the format seen directly by the user.
Is it Normal Form or Normal Format, by the way? You're using the former,
but the doc-comments use the latter.
And is it a phrase we normally use, or was it made up just for this
function? I only ask because this is the first time I've ever run across
it in the context of Subversion, and I'm wondering whether it's the best
choice of words to describe the concept. (But then I'm a database guy
by trade, so 'Normal Form' already means something completely different
to me :-)).
> > But it'd be good to document it here, since we don't
> > use the phrase elsewhere.
>
> Then maybe the README.txt in libsvn_wc should be expanded, or some
> document in notes/ should be added explaining what the normal form is
> defined as. All a programmer should need to know (IMO) is that we
> have such a thing and that you need to translate a WC file to it
> before sending it to the repository.
>
To be clear: "it" in my sentence meant just an explanation of what
the phrase meant, and why we need to care. A document in notes/ would
probably be helpful too, but my main concern was for consumers of the API.
> > (Hmm, would something even clearer like
> > SVN_WC_TRANSLATE_TEXTBASE_TO_WORKING be better? Probably too long.)
>
> But: it's not only the text base, it's also the contents as stored in
> the repository.
>
(which we should make clear (briefly) when giving an overview of the API.)
> > > + /** Only do translation associated with the svn:special property only */
> > Offhand, I don't know what kind of translation we'd do with respect to the
> > svn:special property at all. Could we have some more information here?
>
> Again, I agree that we may need more information on what libsvn_wc
> does and how, but I'd rather put that in some design document than in
> the docstrings.
>
Ok, fair enough. You might mention the work 'symlink' somewhere,
to give the reader a fighting chance of working out what's going on.
Like Julian, I also had trouble working out that this was actually a
_subtractive_ option: it prevents other stuff from happening. (Is it
really needed as an option? What's the use case?)
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 11/17/05, Julian Foad <ju...@btopenworld.com> wrote:
> Erik Huelsmann wrote:
> > On 11/15/05, Malcolm Rowe <ma...@farside.org.uk> wrote:
> >
> >>>+ /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> >>>+#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
> >>>+
> >>>+ /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
> >>>+#define SVN_WC_TRANSLATE_TO_NF 0x00000001
> >>
> >>What is 'Normal Form(at)'?
> >
> > Well, the essence of my changes are actually to keep that an
> > implementation detail. The user of these constants should not have to
> > know what it is.
>
> I think Malcolm probably meant (or at least it would be reasonable to ask) not
> "What are the exact details of Normal Format", but "What is the fundamental
> definition of Normal Format", so the answer would be something like:
Yes, I'm adding it to my followup commit to at least fix these errors
- whatever we decide to do with the function in the end.
> "Normal Format of a given file is the format in which the file is stored in
> the repository and as the pristine copy of its base revision in the working
> copy. [...Details about why it is used and how it can differ from the working
> copy...]"
>
> I agree with your comment that such information should be somewhere like in a
> libsvn_wc documentation file.
Since it's more of a system wide issue (not libsvn_wc only), I think
I'll need to write a document for notes/ and refer to that.
bye,
Erik.
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Julian Foad <ju...@btopenworld.com>.
Erik Huelsmann wrote:
> On 11/15/05, Malcolm Rowe <ma...@farside.org.uk> wrote:
>
>>>+ /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
>>>+#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
>>>+
>>>+ /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
>>>+#define SVN_WC_TRANSLATE_TO_NF 0x00000001
>>
>>What is 'Normal Form(at)'?
>
> Well, the essence of my changes are actually to keep that an
> implementation detail. The user of these constants should not have to
> know what it is.
I think Malcolm probably meant (or at least it would be reasonable to ask) not
"What are the exact details of Normal Format", but "What is the fundamental
definition of Normal Format", so the answer would be something like:
"Normal Format of a given file is the format in which the file is stored in
the repository and as the pristine copy of its base revision in the working
copy. [...Details about why it is used and how it can differ from the working
copy...]"
I agree with your comment that such information should be somewhere like in a
libsvn_wc documentation file.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r17362 - in trunk/subversion: libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 11/15/05, Malcolm Rowe <ma...@farside.org.uk> wrote:
> Couple of comments on the documentation changes (only), inline:
>
> On Tue, Nov 15, 2005 at 01:36:57PM -0600, dionisos@tigris.org wrote:
> > Author: dionisos
> > Date: Tue Nov 15 13:36:57 2005
> > New Revision: 17362
> >
>
> > * subversion/libsvn_wc/update_editor.c
> > Update callers to svn_wc_translated_file and
> > svn_wc_translated_file2, removing superseeded static functions.
> "superseded"
>
> > --- trunk/subversion/include/svn_wc.h (original)
> > +++ trunk/subversion/include/svn_wc.h Tue Nov 15 13:36:57 2005
> > + /** Translate from Normal Format; excludes SVN_WC_TRANSLATE_TO_NF */
> > +#define SVN_WC_TRANSLATE_FROM_NF 0x00000000
> > +
> > + /** Translate to Normal Format; excludes SVN_WC_TRANSLATE_FROM_NF */
> > +#define SVN_WC_TRANSLATE_TO_NF 0x00000001
>
> What is 'Normal Form(at)'?
Well, the essence of my changes are actually to keep that an
implementation detail. The user of these constants should not have to
know what it is.
> I guess (from later) that it's the format
> that the text-base is stored in: LF line termination and unexpanded
> keywords.
Ah! See? This is exactly why it should be a (hidden) implementation
fact: you have it only partly right. Yes, some files have LF
termination. Others have CRLF termination as their normal form.
Keywords are always stored unexpanded. But, the normal form for a
symlink is entirely different from the actual versioned file (ie not a
symlink).
> But it'd be good to document it here, since we don't
> use the phrase elsewhere.
Then maybe the README.txt in libsvn_wc should be expanded, or some
document in notes/ should be added explaining what the normal form is
defined as. All a programmer should need to know (IMO) is that we
have such a thing and that you need to translate a WC file to it
before sending it to the repository.
> (Hmm, would something even clearer like
> SVN_WC_TRANSLATE_TEXTBASE_TO_WORKING be better? Probably too long.)
But: it's not only the text base, it's also the contents as stored in
the repository.
> > +
> > + /** Only do translation associated with the svn:special property only */
> > +#define SVN_WC_TRANSLATE_SPECIAL_ONLY 0x00000002
>
> Offhand, I don't know what kind of translation we'd do with respect to the
> svn:special property at all. Could we have some more information here?
Again, I agree that we may need more information on what libsvn_wc
does and how, but I'd rather put that in some design document than in
the docstrings.
> > +
> > + /** Translate the special property only */
> > +#define SVN_WC_TRANSLATE_DEL_TMP_ON_POOL_CLEANUP 0x00000004
>
> Comment is wrong.
Oops! Thanks!
> > +/** Set @a *xlated_path to a path to translated copy of @src
> > + * or to @a src itself if no translation is necessary.
> "(unless SVN_WC_TRANSLATE_FORCE_COPY is set)"? I know, you mention it
> further down, maybe it's unimportant.
>
> > + * If @c SVN_WC_TRANSLATE_DEL_TEMP_ON_POOL_CLEANUP is specified,
> > + * a pool cleanup handler is registered on *xlated_p if it points to a
> > + * temporary file.
> > *
> > * If an error is returned, the effect on @a *xlated_p is undefined.
>
> "xlated_path", both times.
Thanks for the review! What do you think about my comments?
bye,
Erik.