Re: [PATCH 2/4] remove_index_entry_at: move documentation to cache.h

2017-01-18 Thread Stefan Beller
On Wed, Jan 18, 2017 at 1:16 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Signed-off-by: Stefan Beller 
>> ---
>>  cache.h  | 3 +++
>>  read-cache.c | 1 -
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 270a0d0ea7..26632065a5 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -599,7 +599,10 @@ extern int index_name_pos(const struct index_state *, 
>> const char *name, int name
>>  #define ADD_CACHE_KEEP_CACHE_TREE 32 /* Do not invalidate cache-tree */
>>  extern int add_index_entry(struct index_state *, struct cache_entry *ce, 
>> int option);
>>  extern void rename_index_entry_at(struct index_state *, int pos, const char 
>> *new_name);
>> +
>> +/* Remove entry, return 1 if there are more entries after pos. */
>>  extern int remove_index_entry_at(struct index_state *, int pos);
>
> What is the reason why this now promise to return 1, as opposed to
> the original that were allowed to return anything that is "true"?
> Is it because you are adding other return values that mean different
> things?
>
> If that is the case it may be fine (it depends on what these other
> values mean and what use case it supports), but please do that in a
> separate patch.
>

Actually my line of thinking was to improve the correctness by being more
specific.

In a reroll I move the comment verbatim.


Re: [PATCH 2/4] remove_index_entry_at: move documentation to cache.h

2017-01-18 Thread Junio C Hamano
Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  cache.h  | 3 +++
>  read-cache.c | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cache.h b/cache.h
> index 270a0d0ea7..26632065a5 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -599,7 +599,10 @@ extern int index_name_pos(const struct index_state *, 
> const char *name, int name
>  #define ADD_CACHE_KEEP_CACHE_TREE 32 /* Do not invalidate cache-tree */
>  extern int add_index_entry(struct index_state *, struct cache_entry *ce, int 
> option);
>  extern void rename_index_entry_at(struct index_state *, int pos, const char 
> *new_name);
> +
> +/* Remove entry, return 1 if there are more entries after pos. */
>  extern int remove_index_entry_at(struct index_state *, int pos);

What is the reason why this now promise to return 1, as opposed to
the original that were allowed to return anything that is "true"?
Is it because you are adding other return values that mean different
things?  

If that is the case it may be fine (it depends on what these other
values mean and what use case it supports), but please do that in a
separate patch.

> +
>  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
> diff --git a/read-cache.c b/read-cache.c
> index 2eca639cce..63a414cdb5 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, 
> const char *name, int namel
>   return index_name_stage_pos(istate, name, namelen, 0);
>  }
>  
> -/* Remove entry, return true if there are more entries to go.. */
>  int remove_index_entry_at(struct index_state *istate, int pos)
>  {
>   struct cache_entry *ce = istate->cache[pos];


[PATCH 2/4] remove_index_entry_at: move documentation to cache.h

2017-01-17 Thread Stefan Beller
Signed-off-by: Stefan Beller 
---
 cache.h  | 3 +++
 read-cache.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 270a0d0ea7..26632065a5 100644
--- a/cache.h
+++ b/cache.h
@@ -599,7 +599,10 @@ extern int index_name_pos(const struct index_state *, 
const char *name, int name
 #define ADD_CACHE_KEEP_CACHE_TREE 32   /* Do not invalidate cache-tree */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int 
option);
 extern void rename_index_entry_at(struct index_state *, int pos, const char 
*new_name);
+
+/* Remove entry, return 1 if there are more entries after pos. */
 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
diff --git a/read-cache.c b/read-cache.c
index 2eca639cce..63a414cdb5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, const 
char *name, int namel
return index_name_stage_pos(istate, name, namelen, 0);
 }
 
-/* Remove entry, return true if there are more entries to go.. */
 int remove_index_entry_at(struct index_state *istate, int pos)
 {
struct cache_entry *ce = istate->cache[pos];
-- 
2.11.0.299.g762782ba8a