Re: [PATCH v6 14/16] rebase-interactive: rewrite edit_todo_list() to handle the initial edit

[Alban Gruin <alban.gruin@gmail.com>]

Hi Phillip,

Le 01/02/2019 à 12:03, Phillip Wood a écrit :
>>       }
>>   -    strbuf_reset(&todo_list.buf);
>> -    if (launch_sequence_editor(todo_file, &todo_list.buf, NULL)) {
>> -        todo_list_release(&todo_list);
>> -        return -1;
>> -    }
>> +    if (launch_sequence_editor(todo_file, &new_todo->buf, NULL))
>> +        return -2;
>>   -    if (!todo_list_parse_insn_buffer(r, todo_list.buf.buf,
>> &todo_list))
>> -        res = todo_list_write_to_file(r, &todo_list, todo_file, NULL,
>> NULL, -1,
>> -                          flags & ~(TODO_LIST_SHORTEN_IDS));
>> +    strbuf_stripspace(&new_todo->buf, 1);
>> +    if (initial && new_todo->buf.len == 0)
>> +        return -3;
>>   -    todo_list_release(&todo_list);
>> -    return res;
>> +    if (!initial)
>> +        todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo);
> 
> error handling. Also why don't we try parse the file for the initial
> edit - is it done somewhere else?
> 

Yes, it’s done in complete_action().

-- Alban

> Best Wishes
> 
> Phillip
> 
>> +
>> +    return 0;
>>   }
>>     define_commit_slab(commit_seen, unsigned char);
>> diff --git a/rebase-interactive.h b/rebase-interactive.h
>> index 0e5925e3aa..44dbb06311 100644
>> --- a/rebase-interactive.h
>> +++ b/rebase-interactive.h
>> @@ -8,7 +8,9 @@ struct todo_list;
>>   void append_todo_help(unsigned keep_empty, int command_count,
>>                 const char *shortrevisions, const char *shortonto,
>>                 struct strbuf *buf);
>> -int edit_todo_list(struct repository *r, unsigned flags);
>> +int edit_todo_list(struct repository *r, struct todo_list *todo_list,
>> +           struct todo_list *new_todo, const char *shortrevisions,
>> +           const char *shortonto, unsigned flags);
>>   int todo_list_check(struct todo_list *old_todo, struct todo_list
>> *new_todo);
>>     #endif
>> diff --git a/sequencer.c b/sequencer.c
>> index 92de982bc4..8f47f0cf39 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -55,8 +55,7 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
>>    * file and written to the tail of 'done'.
>>    */
>>   GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
>> -static GIT_PATH_FUNC(rebase_path_todo_backup,
>> -             "rebase-merge/git-rebase-todo.backup")
>> +GIT_PATH_FUNC(rebase_path_todo_backup,
>> "rebase-merge/git-rebase-todo.backup")
>>     /*
>>    * The rebase command lines that have already been processed. A line
>> diff --git a/sequencer.h b/sequencer.h
>> index c5bee8124c..68acab980b 100644
>> --- a/sequencer.h
>> +++ b/sequencer.h
>> @@ -10,6 +10,7 @@ struct repository;
>>   const char *git_path_commit_editmsg(void);
>>   const char *git_path_seq_dir(void);
>>   const char *rebase_path_todo(void);
>> +const char *rebase_path_todo_backup(void);
>>     #define APPEND_SIGNOFF_DEDUP (1u << 0)
>>  
>