Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes: And I do not think an array of things that are operated on should not be named ref_filter_item. Is the double-negation intended? It seems contradictory with: Surely, the latter set of operations to be applied may currently be only filtering, but who

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Matthieu Moy
karthik nayak karthik@gmail.com writes: At some point, I'd expect something like filtered_list_of_refs = filer(full_list_of_refs, description_of_filter); That would remove some refs from full_list_of_refs according to description_of_filter. (totally invented code, only to show the

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Matthieu Moy
Christian Couder christian.cou...@gmail.com writes: struct ref_list { int count, alloc; struct ref_filter_item **items; const char **name_patterns; }; Matthieu, I think you forgot to remove const char **name_patterns; in the above struct, as you put it in the

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Christian Couder
On Sat, May 23, 2015 at 4:42 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: karthik nayak karthik@gmail.com writes: At some point, I'd expect something like filtered_list_of_refs = filer(full_list_of_refs, description_of_filter); That would remove some refs from

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: But I think this could be clearer in the code (and/or comment + commit message). Perhaps stg like: struct ref_filter_data /* Probably not the best name */ { struct ref_list list; struct ref_filter filter; }; struct

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-23 Thread Karthik Nayak
But it also contains struct ref_filter_item **items, which as I understand it contains a list of refs (with name, sha1 such). That's the part I do not find natural: the same structure contains both the list of refs and the way it should be filtered. Re-reading the patch, I seem to understand

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-22 Thread Matthieu Moy
karthik nayak karthik@gmail.com writes: I miss a high-level description of what the code is doing. Essentially, there's the complete repository list of refs, and you want to filter only some of them, right? From the name, I would guess that ref_filter is the structure describing how

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-22 Thread karthik nayak
On 05/22/2015 12:14 PM, Matthieu Moy wrote: karthik nayak karthik@gmail.com writes: I miss a high-level description of what the code is doing. Essentially, there's the complete repository list of refs, and you want to filter only some of them, right? From the name, I would guess that

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-22 Thread karthik nayak
On 05/22/2015 12:10 AM, Eric Sunshine wrote: On Thu, May 21, 2015 at 1:30 PM, karthik nayak karthik@gmail.com wrote: On 05/21/2015 12:37 AM, Eric Sunshine wrote: On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: Makefile | 1 + ref-filter.c | 73

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes: +static int match_name_as_path(const char **pattern, const char *refname) I would have appreciated a short docstring. The full doc would probably be as long as the code, but a few examples of what matches and what doesn't can help the reader.

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread karthik nayak
I miss a high-level description of what the code is doing. Essentially, there's the complete repository list of refs, and you want to filter only some of them, right? From the name, I would guess that ref_filter is the structure describing how you are filtering, but from the code it seems to

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 1:30 PM, karthik nayak karthik@gmail.com wrote: On 05/21/2015 12:37 AM, Eric Sunshine wrote: On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: Makefile | 1 + ref-filter.c | 73

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread karthik nayak
On 05/21/2015 02:17 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: +static int match_name_as_path(const char **pattern, const char *refname) I would have appreciated a short docstring. The full doc would probably be as long as the code, but a few examples of what

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread karthik nayak
On 05/21/2015 12:37 AM, Eric Sunshine wrote: On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: add a ref-filter API to provide functions to filter refs for listing. This will act as a common library for commands like 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter

Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-20 Thread Eric Sunshine
On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: add a ref-filter API to provide functions to filter refs for listing. This will act as a common library for commands like 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter will enable each of these commands to