Re: [PATCH 02/22] sequencer: use memoized sequencer directory path
Hi Kuba,
On Mon, 29 Aug 2016, Jakub Narębski wrote:
> W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
> > Signed-off-by: Johannes Schindelin
> > ---
> > 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).
I agree, but I did not find any way to reorder this without substantial
manual work...
> > 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.
The idea is that this path is declared and defined in the sequencer. There
are other call sites, too, so they have to be changed at the same time...
I'd really like to keep the "sequencer:" prefix because it is semantically
correct: this change is about the sequencer, not about the other call
sites.
Ciao,
Johannes
Re: [PATCH 02/22] sequencer: use memoized sequencer directory path
W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
> Signed-off-by: Johannes Schindelin
> ---
> 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).
> 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.
> 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.
>
> 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)
>
>
[PATCH 02/22] sequencer: use memoized sequencer directory path
Signed-off-by: Johannes Schindelin
---
builtin/commit.c | 2 +-
sequencer.c | 11 ++-
sequencer.h | 5 +
3 files changed, 8 insertions(+), 10 deletions(-)
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
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")
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);
#define APPEND_SIGNOFF_DEDUP (1u << 0)
--
2.10.0.rc1.114.g2bd6b38
