Re: [PATCH 1/4] clarify %f documentation

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2016, 21:33 UTC
Message-ID: <xmqq8ty44y4t.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160616203259.5886-2-joeyh@joeyh.name>

Joey Hess <joeyh@joeyh.name> writes:

> It's natural to expect %f to be an actual file on disk; help avoid that
> mistake.

I agree that "the name of the file" can be interpreted in many ways, and I agree that it would be a good idea to find a better phrase to name the path that is being worked on, but I do not think "the file in the git repository" is that phrase. When I first read the updated text without the above two lines in the log message, I thought "hmph, so we may use a temporary file somewhere in $GIT_DIR/ and that would be pointed at by %f, not the actual path we are working on???", i.e. the rephrasing had exactly the opposite effect on me.

Given that this being part of gitattributes(5) that begins with

    A `gitattributes` file is a simple text file that gives
    `attributes` to pathnames.

    Each line in `gitattributes` file is of form:

            pattern	attr1 attr2 ...

    That is, a pattern followed by an attributes list,
    separated by whitespaces.  When the pattern matches the
    path in question, the attributes listed on the line are given to
    the path.

and that the readers already read something like this in the paragraphs before the mention of '%f':

    For example, in .gitattributes, you would assign the `filter`
    attribute for paths.

    ------------------------
    *.c	filter=indent
    ------------------------

    Then you would define ...

I think using the word "path" somewhere in the updated description is more likely to have the effect you desire.

Show 30 quoted lines

> Signed-off-by: Joey Hess <joeyh@joeyh.name>
> ---
>  Documentation/gitattributes.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index e3b1de8..e077cc9 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -365,8 +365,8 @@ you can declare that the filter is `required`, in the configuration:
>  ------------------------
>  
>  Sequence "%f" on the filter command line is replaced with the name of
> -the file the filter is working on.  A filter might use this in keyword
> -substitution.  For example:
> +the file in the git repository the filter is working on.
> +A filter might use this in keyword substitution.  For example:
>  
>  ------------------------
>  [filter "p4"]
> @@ -374,6 +374,9 @@ substitution.  For example:
>  	smudge = git-p4-filter --smudge %f
>  ------------------------
>  
> +Note that the "%f" is the name of a file in the git repository; the
> +corresponding file on disk may not exist, or may have unrelated contents to
> +what git is filtering.
>  
>  Interaction between checkin/checkout attributes
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^