Hyrum Wright wrote:
> On Tue, Sep 21, 2010 at 10:25 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> > hwright_at_apache.org wrote on Fri, Sep 17, 2010 at 20:04:53 -0000:
> >> * subversion/include/svn_client.h
> >> (svn_client_cat2): Rewrite docstring (see r997639).
[...]
> >> *
> >> * A brief word on operative and peg revisions.
> >> *
> >> + * If the kind of the peg revision is #svn_opt_revision_unspecified, then it
> >> + * defaults to #svn_opt_revision_head for URLs and #svn_opt_revision_working
> >> + * for local paths.
> >> + *
> >> * For deeper insight, please see the
> >> * <a href="http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html">
> >> * Peg and Operative Revisions</a> section of the Subversion Book.
> >> @@ -4687,26 +4691,30 @@ svn_client_ls(apr_hash_t **dirents,
> >> */
> >>
> >> /**
> >> - * Output the content of file identified by @a path_or_url and @a
> >> - * revision to the stream @a out. The actual node revision selected
> >> - * is determined by the path as it exists in @a peg_revision. If @a
> >> - * peg_revision->kind is #svn_opt_revision_unspecified, then it defaults
> >> - * to #svn_opt_revision_head for URLs or #svn_opt_revision_working
> >> - * for WC targets.
> >> - *
> >> - * If @a path_or_url is not a local path, then if @a revision is of
> >> - * kind #svn_opt_revision_previous (or some other kind that requires
> >> - * a local path), an error will be returned, because the desired
> >> - * revision cannot be determined.
> >> - *
> >> - * Use the authentication baton cached in @a ctx to authenticate against the
> >> - * repository.
> >> + * Output the content of a file.
> >> + *
> >> + * @param[in] out The stream to which the content will be written.
> >> + * @param[in] path_or_url The path or URL of the file.
> >> + * @param[in] peg_revision The peg revision.
> >> + * @param[in] revision The operative revision.
> >> + * @param[in] ctx The standard client context, used for possible
> >> + * authentication.
> >> + * @param[in] pool Used for any temporary allocation.
> >> *
> >> - * Perform all allocations from @a pool.
> >
> > Sorry, but I like the old style better. I could read it and actually
> > understand what the function does; whereas text like
> >
> > * Output the content of a file.
Although the concise style appears to contain just as much information
when read in its entirety, I think we have indeed lost something
semantically in this translation. I think the reader of this
description is liable to think:
"Output? To the screen, or what?"
"A file? Any old file, or what?"
The reader then has to read the list of parameters to find out. In this
case the first param happens to say that a stream is used for output,
but the reader isn't sure that there isn't going to be another param
further down the list that says something like "The file to copy to,
which can be supplied instead of or as well as the stream."
Similarly for the fact that it outputs a specified revision of a
(versioned) file; the lack of that piece of information makes the
interpretation of "a file" vague. Simply having a parameter documented
as "The revision" doesn't properly make the connection, although in a
relatively simple case like this the reader can infer that it's the
revision of the file to be output.
The original sentence "Output ... file identified by @a path_or_url and
@a revision ... to a stream" made both of those implications clear.
So perhaps we should aim to say:
* Output the content of a specified revision of a file
* to a stream.
The extra words here would not be fluff or "modifying behaviour", but
rather a more accurate description of the primary behaviour. And this
would give the word "the" something to attach to when we later refer to
"The peg revision", "The stream", and so on.
Daniel, does that address your concerns?
- Julian
> This is *exactly* what the function does, without all the extra fluff.
> Everything else is just modifying behavior, several pieces of which
> are common to many of our APIs, and so should have that documentation
> extracted and linked to. I'm just trying to save the effort of
> reading five paragraphs of prose to find out what a single boolean
> argument does.
>
> The other problem is that the prose can make it hard to pick up on
> inconsistent behavior, when something (such as depth defaults) isn't
> the same between APIs.
>
> > *
> > * @param[in] out The stream to which the content will be written.
> > * @param[in] path_or_url The path or URL of the file.
> > * @param[in] peg_revision The peg revision.
> >
> > tells me virtually nothing more than the signature does.
>
> That being said, improvements are welcome, or we can just revert what
> I've already done.
>
> -Hyrum
Received on 2010-09-22 10:44:08 CEST