Re: [PATCH 07/22] sequencer: future-proof read_populate_todo()
From: Jakub Narębski <hidden>
Date: 2016-08-30 16:08:03
W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
quoted hunk ↗ jump to hunk
Over the next commits, we will work on improving the sequencer to the point where it can process the edit script of an interactive rebase. To that end, we will need to teach the sequencer to read interactive rebase's todo file. In preparation, we consolidate all places where that todo file is needed to call a function that we will later extend. Signed-off-by: Johannes Schindelin <redacted> --- sequencer.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)diff --git a/sequencer.c b/sequencer.c index 8d79091..982b6e9 100644 --- a/sequencer.c +++ b/sequencer.c@@ -32,6 +32,11 @@ static const char *get_dir(const struct replay_opts *opts) return git_path_seq_dir(); } +static const char *get_todo_path(const struct replay_opts *opts) +{ + return git_path_todo_file(); +}
I guess that in the future commit the return value of get_todo_path() would change depending on what sequencer is used for, cherry-pick or interactive rebase, that is, contents of replay_opts...
quoted hunk ↗ jump to hunk
+ static int is_rfc2822_line(const char *buf, int len) { int i;@@ -772,25 +777,24 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list, static int read_populate_todo(struct commit_list **todo_list, struct replay_opts *opts) { + const char *todo_file = get_todo_path(opts);
...and that's why you have added this temporary variable here, to not repeat get_todo_path(opts) calculations...
struct strbuf buf = STRBUF_INIT;
int fd, res;
- fd = open(git_path_todo_file(), O_RDONLY);
+ fd = open(todo_file, O_RDONLY);
if (fd < 0)
- return error_errno(_("Could not open %s"),
- git_path_todo_file());
+ return error_errno(_("Could not open %s"), todo_file);... So that's why it is s/git_path_todo_file()/todo_file/ replacement, and not simply...
quoted hunk ↗ jump to hunk
if (strbuf_read(&buf, fd, 0) < 0) { close(fd); strbuf_release(&buf); - return error(_("Could not read %s."), git_path_todo_file()); + return error(_("Could not read %s."), todo_file); } close(fd); res = parse_insn_buffer(buf.buf, todo_list, opts); strbuf_release(&buf); if (res) - return error(_("Unusable instruction sheet: %s"), - git_path_todo_file()); + return error(_("Unusable instruction sheet: %s"), todo_file); return 0; }@@ -1064,7 +1068,7 @@ static int sequencer_continue(struct replay_opts *opts) { struct commit_list *todo_list = NULL; - if (!file_exists(git_path_todo_file())) + if (!file_exists(get_todo_path(opts)))
...the s/git_path_todo_file()/git_todo_path(opts)/, isn't it?
return continue_single_pick(); if (read_populate_opts(opts) || read_populate_todo(&todo_list, opts))
Looks good; though I have not checked if all calling sites were converted. Good work, -- Jakub Narębski