Re: [PATCH 02/22] sequencer: use memoized sequencer directory path

From: Jakub Narębski <jnareb@gmail.com>
Date: Aug 29, 2016, 19:54 UTC
Message-ID: <10e19839-2875-2610-0af3-0b86831916f0@gmail.com>
In-Reply-To: <d032a9d765aaca0655b1e363e67d9bf24864396c.1472457609.git.johannes.schindelin@gmx.de>

W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:

Show 6 quoted lines

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/commit.c |  2 +-
>  sequencer.c      | 11 ++++++-----
>  sequencer.h      |  5 +----
>  3 files changed, 8 insertions(+), 10 deletions(-)

Just a sidenote: it would be probably easier to read with *.h before *.c (at least this particular one).

Show 13 quoted lines

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 77e3dc8..0221190 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -173,7 +173,7 @@ static void determine_whence(struct wt_status *s)
>  		whence = FROM_MERGE;
>  	else if (file_exists(git_path_cherry_pick_head())) {
>  		whence = FROM_CHERRY_PICK;
> -		if (file_exists(git_path(SEQ_DIR)))
> +		if (file_exists(git_path_seq_dir()))
>  			sequencer_in_use = 1;
>  	}
>  	else

So it is more "Use memoized sequencer directory path" rather than "sequencer: use memoized sequencer directory path" - it replaces all occurrences of SEQ_DIR,... that's why it can be removed from 'sequencer.h'.

Though perhaps I misunderstood "sequencer: " prefix there. Don't mind me then.

Show 17 quoted lines

> diff --git a/sequencer.c b/sequencer.c
> index b6481bb..4d2b4e3 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -21,10 +21,11 @@
>  const char sign_off_header[] = "Signed-off-by: ";
>  static const char cherry_picked_prefix[] = "(cherry picked from commit ";
>  
> -static GIT_PATH_FUNC(git_path_todo_file, SEQ_TODO_FILE)
> -static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE)
> -static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR)
> -static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_FILE)
> +GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
> +
> +static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
> +static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
> +static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")

This also makes the ordering of memoized-path variables more sensible. Good work.

Show 25 quoted lines

>  
>  static int is_rfc2822_line(const char *buf, int len)
>  {
> @@ -112,7 +113,7 @@ static void remove_sequencer_state(void)
>  {
>  	struct strbuf seq_dir = STRBUF_INIT;
>  
> -	strbuf_addstr(&seq_dir, git_path(SEQ_DIR));
> +	strbuf_addstr(&seq_dir, git_path_seq_dir());
>  	remove_dir_recursively(&seq_dir, 0);
>  	strbuf_release(&seq_dir);
>  }
> diff --git a/sequencer.h b/sequencer.h
> index 2ca096b..c955594 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -1,10 +1,7 @@
>  #ifndef SEQUENCER_H
>  #define SEQUENCER_H
>  
> -#define SEQ_DIR		"sequencer"
> -#define SEQ_HEAD_FILE	"sequencer/head"
> -#define SEQ_TODO_FILE	"sequencer/todo"
> -#define SEQ_OPTS_FILE	"sequencer/opts"
> +const char *git_path_seq_dir(void);

Right, I see this matches other git_path_*() functions declared in cache.h

>  
>  #define APPEND_SIGNOFF_DEDUP (1u << 0)
>  
>