Re: [PATCH 6/7] Export rerere() and launch_editor().

["Kristian Høgsberg" <krh@redhat.com>]

On Wed, 2007-09-19 at 16:52 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > +/* Export for builtin-commit. */
> > +int rerere(void)
> > +{
> > +	struct path_list merge_rr = { NULL, 0, 0, 1 };
> > +	int fd;
> > +
> > +	git_config(git_rerere_config);
> > +	if (!is_rerere_enabled())
> > +		return 0;
> > +
> > +	merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
> > +	fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
> > +	read_rr(&merge_rr);
> > +	return do_plain_rerere(&merge_rr, fd);
> > +}
> 
> Is it just me who sees a suboptimal cut and paste here?
> 
> BTW, [1-5/7] look good so far.
> 
> diff --git a/builtin-rerere.c b/builtin-rerere.c
> index 29d057c..2f51ae0 100644
> --- a/builtin-rerere.c
> +++ b/builtin-rerere.c
> @@ -415,18 +415,39 @@ static int is_rerere_enabled(void)
>  	return 1;
>  }
>  
> -int cmd_rerere(int argc, const char **argv, const char *prefix)
> +static int setup_rerere(struct path_list *merge_rr)
>  {
> -	struct path_list merge_rr = { NULL, 0, 0, 1 };
> -	int i, fd = -1;
> +	int fd;
>  
>  	git_config(git_rerere_config);
>  	if (!is_rerere_enabled())
> -		return 0;
> +		return -1;
>  
>  	merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
>  	fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
> -	read_rr(&merge_rr);
> +	read_rr(merge_rr);
> +	return fd;
> +}
> +
> +int rerere(void)
> +{
> +	struct path_list merge_rr = { NULL, 0, 0, 1 };
> +	int fd;
> +
> +	fd = setup_rerere(&merge_rr);
> +	if (fd < 0)
> +		return 0;
> +	return do_plain_rerere(&merge_rr, fd);
> +}
> +
> +int cmd_rerere(int argc, const char **argv, const char *prefix)
> +{
> +	struct path_list merge_rr = { NULL, 0, 0, 1 };
> +	int i, fd;
> +
> +	fd = setup_rerere(&merge_rr);
> +	if (fd < 0)
> +		return 0;
>  
>  	if (argc < 2)
>  		return do_plain_rerere(&merge_rr, fd);

That looks better, yes.

Kristian