Re: [PATCH v2 03/20] refs_ref_iterator_begin(): new function

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> Extract a new function from `do_for_each_ref()`. It will be useful
> elsewhere.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs.c               | 15 +++++++++++++--
>  refs/refs-internal.h | 11 +++++++++++
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 0ed6c3c7a4..aeb704ab92 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)
>         return head_ref_submodule(NULL, fn, cb_data);
>  }
>
> +struct ref_iterator *refs_ref_iterator_begin(
> +               struct ref_store *refs,
> +               const char *prefix, int trim, int flags)
> +{
> +       struct ref_iterator *iter;
> +
> +       iter = refs->be->iterator_begin(refs, prefix, flags);
> +       iter = prefix_ref_iterator_begin(iter, prefix, trim);

Off topic. This code made me wonder if we really need the prefix
iterator if prefix is NULL. And in fact we don't since
prefix_ref_iterator_begin() will return the old iter in that case. But
it's probably better to move that optimization outside. I think it's
easier to understand that way, calling prefix_ref_ will always give
you a new iterator. Don't call it unless you want to have it.

> +/*
> + * Return an iterator that goes over each reference in `refs` for
> + * which the refname begins with prefix. If trim is non-zero, then
> + * trim that many characters off the beginning of each refname. flags
> + * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in
> + * the iteration.
> + */

Do we need a separate docstring here? I think we document more or less
the same for ref_iterator_begin_fn (except the include-broken flag).

> +struct ref_iterator *refs_ref_iterator_begin(
> +               struct ref_store *refs,
> +               const char *prefix, int trim, int flags);
> +
-- 
Duy