Re: [PATCH 3/4] document add_[file_]to_index

> Stefan Beller <sbeller@google.com> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  cache.h | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 26632065a5..acc639d6e0 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -605,13 +605,20 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>>
>>  extern void remove_marked_cache_entries(struct index_state *istate);
>>  extern int remove_file_from_index(struct index_state *, const char *path);
>> -#define ADD_CACHE_VERBOSE 1
>> -#define ADD_CACHE_PRETEND 2
>> -#define ADD_CACHE_IGNORE_ERRORS      4
>> -#define ADD_CACHE_IGNORE_REMOVAL 8
>> -#define ADD_CACHE_INTENT 16
>> +
>> +#define ADD_CACHE_VERBOSE 1          /* verbose */
>> +#define ADD_CACHE_PRETEND 2          /* dry run */
>> +#define ADD_CACHE_IGNORE_ERRORS 4    /* ignore errors */
>> +#define ADD_CACHE_IGNORE_REMOVAL 8   /* do not remove files from index */
>> +#define ADD_CACHE_INTENT 16          /* intend to add later; stage empty file */
>
> These repeat pretty much the same thing, which is an indication that
> the macro names are chosen well not to require extraneous comments
> like these, no?

>
>> +/*
>> + * Adds the given path the index, respecting the repsitory configuration, e.g.
>> + * in case insensitive file systems, the path is normalized.
>> + */
>>  extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
>
> s/repsitory/repository/;
>
>> +/* stat the file then call add_to_index */
>>  extern int add_file_to_index(struct index_state *, const char *path, int flags);
>> +
>
> As you do not say "use the provided stat info to mark the cache
> entry up-to-date" in the add_to_index(), I am not sure if mentioning
> "stat the file then" has much value.  Besides, you are supposed to
> lstat(2) the file, not "stat", no?
>
> I'd cover these two under the same heading and comment if I were
> doing this.
>
>         These two are used to add the contents of the file at path
>         to the index, marking the working tree up-to-date by storing
>         the cached stat info in the resulting cache entry.  A caller
>         that has already run lstat(2) on the path can call
>         add_to_index(), and all others can call add_file_to_index();
>         the latter will do necessary lstat(2) internally before
>         calling the former.
>
> or something along that line.