W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
> 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 <johannes.schindelin@xxxxxx>
> ---
> 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...
> +
> 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...
> 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