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

From: Junio C Hamano <[email-masked]>
Date: Wed, 18 Jan 2017 13:22:42 -0800

Stefan Beller <[email-masked]> writes:

> Signed-off-by: Stefan Beller <[email-masked]>
> ---
>  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.

>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
>  extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
>  extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);