Re: [PATCH 2/4] add smudge-to-file and clean-from-file filter configuration

[Junio C Hamano <gitster@pobox.com>]

Joey Hess <joeyh@joeyh.name> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2e1b2e4..bbb9296 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1299,14 +1299,29 @@ format.useAutoBase::
>  	format-patch by default.
>  
>  filter.<driver>.clean::
> -	The command which is used to convert the content of a worktree
> -	file to a blob upon checkin.  See linkgit:gitattributes[5] for
> -	details.
> +	The command which is used on checkin to convert the content of
> +	a worktree file (provided on stdin) to a blob (written to stdout).
> +	See linkgit:gitattributes[5] for details.
>  
>  filter.<driver>.smudge::
> -	The command which is used to convert the content of a blob
> -	object to a worktree file upon checkout.  See
> -	linkgit:gitattributes[5] for details.
> +	The command which is used on checkout to convert the content of a
> +	blob object (provided on stdin) to a worktree file (written to
> +	stdout).
> +	See linkgit:gitattributes[5] for details.

A "filter" by definition reads from its standard input and writes
the result to its standard output, so I do not know if this change
is necessary.  If you are going to do this, in documentation
(i.e. not code), please spell standard input/output out, and avoid
stdin/stdout.

There is what we would want to fix, though.  "worktree file" should
be spelled "working tree file".  This used not to matter before "git
worktree" was invented (before that we used these two terms
interchangeably), but these days the distinction matters.

> +filter.<driver>.clean-from-file::

Documentation/config.txt hopefully lists all the configuration, but
I do not see anything that uses 'words-joined-with-dash' format.
Please do not invent new out-of-convention names.

> +	Optional command which is used on checkin to convert the content
> +	of a worktree file, which can be read from disk, to a blob
> +	(written to stdout).

I am not sure why we want to say "which can be read from disk".

I think what a reader needs to be told (but this paragraph is not
telling her) in order to understand what you meant to say is:

	cleanFromFile is asked to produce the "cleaned" content to
	its standard output (to be stored in the object database),
	but unlike clean, it does not work as a filter and is not
	given a file descriptor to read the working tree contents
	from.  Instead, it is told the path for which the contents
	need to be generated as the first parameter on its command
	line.

(the above is deliberately made verbose and is not meant as a
suggestion to be literally used in your updated documentation).

With that understanding, the reader may be able to guess that "can
be read from disk" is a permission for her cleanFromFile filter
(i.e. it does not necessarily have to read from it and produced its
output by some other magic); otherwise it can be misread as if the
content of a working tree file is deposited on the disk to enable
the filter to read it, but the location of that on-disk file is
somewhere unspecified by this paragraph.

> +	Only used when filter.<driver>.clean is also configured.
> +	See linkgit:gitattributes[5] for details.
> +
> +filter.<driver>.smudge-to-file::
> +	Optional command which is used to convert the content of a blob
> +	object (provided on stdin) to a worktree file, writing directly
> +	to the file.

A similar comment applies.  With ", writing directly to the file."
replaced with something like ". The command is expected to write to
the working tree file at path given as the first parameter on the
command line." it would be sufficient (i.e. it is clear enough that
it does not give the smudged contents via its standard output, and
no need to say "unlike smudge filter, ..." like we needed to for the
"clean" side).