Re: [PATCH v2 10/34] sequencer (rebase -i): allow continuing with staged changes

2016-12-15 Thread Junio C Hamano
Johannes Schindelin  writes:

> When an interactive rebase is interrupted, the user may stage changes
> before continuing, and we need to commit those changes in that case.
>
> Please note that the nested "if" added to the sequencer_continue() is
> not combined into a single "if" because it will be extended with an
> "else" clause in a later patch in this patch series.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  sequencer.c | 40 
>  1 file changed, 40 insertions(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index 80469b6954..855d3ba503 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1829,6 +1829,42 @@ static int continue_single_pick(void)
>   return run_command_v_opt(argv, RUN_GIT_CMD);
>  }
>  
> +static int commit_staged_changes(struct replay_opts *opts)
> +{
> + int amend = 0;
> +
> + if (has_unstaged_changes(1))
> + return error(_("cannot rebase: You have unstaged changes."));

The scripted one from 'master' seems to say

$path_to_the_file: needs update
You must edit all merge conflicts and then
mark them as resolved using git add

when editing an existing commit in this case.  The updated message
looks more sensible for the situation, but I wonder if the control
should even reach at this point.  

One bad thing about reviewing this series is that all the comments
are about codepaths that are not exercised, so they cannot be more
than "they look good".  A comment "If the caller does X, this will
be better than the original" (or this will regress, for that matter)
cannot be validated for its relevance because we won't know the what
the caller does in the endgame while reviewing these earlier steps.





[PATCH v2 10/34] sequencer (rebase -i): allow continuing with staged changes

2016-12-13 Thread Johannes Schindelin
When an interactive rebase is interrupted, the user may stage changes
before continuing, and we need to commit those changes in that case.

Please note that the nested "if" added to the sequencer_continue() is
not combined into a single "if" because it will be extended with an
"else" clause in a later patch in this patch series.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 80469b6954..855d3ba503 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1829,6 +1829,42 @@ static int continue_single_pick(void)
return run_command_v_opt(argv, RUN_GIT_CMD);
 }
 
+static int commit_staged_changes(struct replay_opts *opts)
+{
+   int amend = 0;
+
+   if (has_unstaged_changes(1))
+   return error(_("cannot rebase: You have unstaged changes."));
+   if (!has_uncommitted_changes(0))
+   return 0;
+
+   if (file_exists(rebase_path_amend())) {
+   struct strbuf rev = STRBUF_INIT;
+   unsigned char head[20], to_amend[20];
+
+   if (get_sha1("HEAD", head))
+   return error(_("cannot amend non-existing commit"));
+   if (!read_oneliner(, rebase_path_amend(), 0))
+   return error(_("invalid file: '%s'"), 
rebase_path_amend());
+   if (get_sha1_hex(rev.buf, to_amend))
+   return error(_("invalid contents: '%s'"),
+   rebase_path_amend());
+   if (hashcmp(head, to_amend))
+   return error(_("\nYou have uncommitted changes in your "
+  "working tree. Please, commit them\n"
+  "first and then run 'git rebase "
+  "--continue' again."));
+
+   strbuf_release();
+   amend = 1;
+   }
+
+   if (run_git_commit(rebase_path_message(), opts, 1, 1, amend, 0))
+   return error(_("could not commit staged changes."));
+   unlink(rebase_path_amend());
+   return 0;
+}
+
 int sequencer_continue(struct replay_opts *opts)
 {
struct todo_list todo_list = TODO_LIST_INIT;
@@ -1837,6 +1873,10 @@ int sequencer_continue(struct replay_opts *opts)
if (read_and_refresh_cache(opts))
return -1;
 
+   if (is_rebase_i(opts)) {
+   if (commit_staged_changes(opts))
+   return -1;
+   }
if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
if (read_populate_opts(opts))
-- 
2.11.0.rc3.windows.1