On 06/12/2019 18:24, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
Working out which command wants to create a commit requires detailed
knowledge of the sequencer internals and that knowledge is going to
increase in subsequent commits. With that in mind lets encapsulate that
knowledge in sequencer.c rather than spreading it into builtin/commit.c.
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
builtin/commit.c | 5 +----
sequencer.c | 13 ++++++++++++-
sequencer.h | 3 ++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 3b463522be..d8d4c8e419 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -178,10 +178,7 @@ static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
- else if (file_exists(git_path_cherry_pick_head(the_repository)))
- whence = file_exists(git_path_seq_dir()) ?
- FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
- else
+ else if (!sequencer_determine_whence(the_repository, &whence))
whence = FROM_COMMIT;
if (s)
s->whence = whence;
diff --git a/sequencer.c b/sequencer.c
index 4e0370277b..98e007556c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -40,7 +40,7 @@ static const char cherry_picked_prefix[] = "(cherry picked from commit ";
GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
-GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
+static 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")
@@ -5256,3 +5256,14 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
return 0;
}
+
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
+{
+ if (file_exists(git_path_cherry_pick_head(r))) {
+ *whence = file_exists(git_path_seq_dir()) ?
+ FROM_CHERRY_PICK_MULTI : FROM_CHERRY_PICK_SINGLE;
+ return 1;
+ }
+
+ return 0;
+}
I am not sure if this is a good move---determine_whence() that can
tell not just we are in the middle of cherry-pick (either a single
or multi) but also during a merge may be at the right abstraction
level. Why would we want to invent a separate function that says "I
dunno" during a merge, instead of moving the logic for merge to the
new helper as well? The original determine_whence that takes
wt_status and populates it still has to call the new helper either
way. Also for the matter FROM_COMMIT may also want to be part of
the helper. This all depends on the new callers you plan to invent,
of course.
The idea was for determine_whence() to be able to delegate to the
sequencer to ask if it is doing something without having to expose all
the implementation details of that check in builtin/commit.c. It is
simple enough at this stage but the next patches add more complexity
which would mean exposing various sequencer state files to
builtin/commit.c. This function is only meant to be called from
determine_whence() - callers that want to know if any operations (merge,
cherry-pick etc.) are in progress should be calling that not the
function added here.
Not part of this topic, but the call to file_exists() may want to
become a call to dir_exists() as git-path-seq-dir is clearly a
directory and cannot be a file, right?
Yes
Best Wishes
Phillip
