[PATCH 0/3] Support `git pull --rebase=interactive`

[PATCH 0/3] Support `git pull --rebase=interactive`

[Johannes Schindelin]

A couple of years ago, I found the need to collaborate on topic branches
that were rebased all the time, and I really needed to see what I was
rebasing when pulling, so I introduced an interactively-rebasing pull.

This patch series ports that work to the builtin pull.


Johannes Schindelin (3):
  Teach 'git pull' to handle --rebase=interactive
  Teach 'git remote' that the config var branch.*.rebase can be
    'interactive'
  completion: add missing branch.*.rebase values

 Documentation/config.txt               |  1 +
 Documentation/git-pull.txt             |  4 +++-
 builtin/pull.c                         |  7 ++++++-
 builtin/remote.c                       |  8 ++++++--
 contrib/completion/git-completion.bash |  2 +-
 t/t5520-pull.sh                        | 10 ++++++++++
 6 files changed, 27 insertions(+), 5 deletions(-)

-- 
2.6.3.windows.1.300.g1c25e49

[PATCH 1/3] Teach 'git pull' to handle --rebase=interactive

[Johannes Schindelin]

The way builtin pull works, this incidentally also supports the value
'interactive' for the 'branch.<name>.rebase' config variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt   |  1 +
 Documentation/git-pull.txt |  4 +++-
 builtin/pull.c             |  7 ++++++-
 t/t5520-pull.sh            | 10 ++++++++++
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index beb18da..647b5f0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -865,6 +865,7 @@ branch.<name>.rebase::
 	instead of merging the default branch from the default remote when
 	"git pull" is run. See "pull.rebase" for doing this in a non
 	branch-specific manner.
+	When the value is `interactive`, the rebase is run in interactive mode.
 +
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 93c72a2..a62a2a6 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,7 +101,7 @@ Options related to merging
 include::merge-options.txt[]
 
 -r::
---rebase[=false|true|preserve]::
+--rebase[=false|true|preserve|interactive]::
 	When true, rebase the current branch on top of the upstream
 	branch after fetching. If there is a remote-tracking branch
 	corresponding to the upstream branch and the upstream branch
@@ -113,6 +113,8 @@ to `git rebase` so that locally created merge commits will not be flattened.
 +
 When false, merge the current branch into the upstream branch.
 +
+When `interactive`, enable the interactive mode of rebase.
++
 See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `--rebase` instead of merging.
diff --git a/builtin/pull.c b/builtin/pull.c
index 9e3c738..832d0ad 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -22,7 +22,8 @@ enum rebase_type {
 	REBASE_INVALID = -1,
 	REBASE_FALSE = 0,
 	REBASE_TRUE,
-	REBASE_PRESERVE
+	REBASE_PRESERVE,
+	REBASE_INTERACTIVE
 };
 
 /**
@@ -42,6 +43,8 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
 		return REBASE_TRUE;
 	else if (!strcmp(value, "preserve"))
 		return REBASE_PRESERVE;
+	else if (!strcmp(value, "interactive"))
+		return REBASE_INTERACTIVE;
 
 	if (fatal)
 		die(_("Invalid value for %s: %s"), key, value);
@@ -778,6 +781,8 @@ static int run_rebase(const unsigned char *curr_head,
 	/* Options passed to git-rebase */
 	if (opt_rebase == REBASE_PRESERVE)
 		argv_array_push(&args, "--preserve-merges");
+	else if (opt_rebase == REBASE_INTERACTIVE)
+		argv_array_push(&args, "--interactive");
 	if (opt_diffstat)
 		argv_array_push(&args, opt_diffstat);
 	argv_array_pushv(&args, opt_strategies.argv);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee..c952d5e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -326,6 +326,16 @@ test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
 	test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)"
 '
 
+test_expect_success 'pull.rebase=interactive' '
+	write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
+	echo I was here >fake.out &&
+	false
+	EOF
+	test_set_editor "$TRASH_DIRECTORY/fake-editor" &&
+	test_must_fail git pull --rebase=interactive . copy &&
+	test "I was here" = "$(cat fake.out)"
+'
+
 test_expect_success 'pull.rebase=invalid fails' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase invalid &&
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/remote.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 6694cf2..0af8300 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
 struct branch_info {
 	char *remote_name;
 	struct string_list merge;
-	int rebase;
+	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
 };
 
 static struct string_list branch_list;
@@ -312,6 +312,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 				info->rebase = v;
 			else if (!strcmp(value, "preserve"))
 				info->rebase = 1;
+			else if (!strcmp(value, "interactive"))
+				info->rebase = INTERACTIVE_REBASE;
 		}
 	}
 	return 0;
@@ -980,7 +982,9 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)
 
 	printf("    %-*s ", show_info->width, item->string);
 	if (branch_info->rebase) {
-		printf_ln(_("rebases onto remote %s"), merge->items[0].string);
+		printf_ln(_(branch_info->rebase == INTERACTIVE_REBASE ?
+			"rebases interactively onto remote %s" :
+			"rebases onto remote %s"), merge->items[0].string);
 		return 0;
 	} else if (show_info->any_rebase) {
 		printf_ln(_(" merges with remote %s"), merge->items[0].string);
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH 3/3] completion: add missing branch.*.rebase values

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ab4da7f..51f5223 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1809,7 +1809,7 @@ _git_config ()
 		return
 		;;
 	branch.*.rebase)
-		__gitcomp "false true"
+		__gitcomp "false true preserve interactive"
 		return
 		;;
 	remote.pushdefault)
-- 
2.6.3.windows.1.300.g1c25e49

Re: [PATCH 1/3] Teach 'git pull' to handle --rebase=interactive

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Subject: Re: [PATCH 1/3] Teach 'git pull' to handle --rebase=interactive

Subject: pull: allow interactive rebase with --rebase=interactive

or something?  The same comment applies for the other patches in
this series, and other recent patches from you in general.

> The way builtin pull works, this incidentally also supports the value
> 'interactive' for the 'branch.<name>.rebase' config variable.

That, especially the "incidentally" part, makes it sound as if it is
"because we can easily", not "because it is useful in such and such
way, we must support it".  We definitely want to see patches that
fall into the latter category, and from the sound of 0/3, I think
this one should be advertised as such.

The patch text looks good (admittedly from only a cursory read,
though).

Thanks.

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/remote.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 6694cf2..0af8300 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
>  struct branch_info {
>  	char *remote_name;
>  	struct string_list merge;
> -	int rebase;
> +	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
>  };
>  
>  static struct string_list branch_list;
> @@ -312,6 +312,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
>  				info->rebase = v;
>  			else if (!strcmp(value, "preserve"))
>  				info->rebase = 1;

This should become NORMAL_REBASE, I would think, even though the
resulting machine code should be identical.

> +			else if (!strcmp(value, "interactive"))
> +				info->rebase = INTERACTIVE_REBASE;
>  		}
>  	}
>  	return 0;
> @@ -980,7 +982,9 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)
>  
>  	printf("    %-*s ", show_info->width, item->string);
>  	if (branch_info->rebase) {
> -		printf_ln(_("rebases onto remote %s"), merge->items[0].string);
> +		printf_ln(_(branch_info->rebase == INTERACTIVE_REBASE ?
> +			"rebases interactively onto remote %s" :
> +			"rebases onto remote %s"), merge->items[0].string);
>  		return 0;
>  	} else if (show_info->any_rebase) {
>  		printf_ln(_(" merges with remote %s"), merge->items[0].string);

Re: [PATCH 3/3] completion: add missing branch.*.rebase values

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ab4da7f..51f5223 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1809,7 +1809,7 @@ _git_config ()
>  		return
>  		;;
>  	branch.*.rebase)
> -		__gitcomp "false true"
> +		__gitcomp "false true preserve interactive"
>  		return
>  		;;
>  	remote.pushdefault)

OK, thanks.

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Johannes Schindelin]

Hi Junio,

On Tue, 12 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  builtin/remote.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/remote.c b/builtin/remote.c
> > index 6694cf2..0af8300 100644
> > --- a/builtin/remote.c
> > +++ b/builtin/remote.c
> > @@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
> >  struct branch_info {
> >  	char *remote_name;
> >  	struct string_list merge;
> > -	int rebase;
> > +	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
> >  };
> >  
> >  static struct string_list branch_list;
> > @@ -312,6 +312,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
> >  				info->rebase = v;
> >  			else if (!strcmp(value, "preserve"))
> >  				info->rebase = 1;
> 
> This should become NORMAL_REBASE, I would think, even though the
> resulting machine code should be identical.

Fixed in v2.

Thanks,
Dscho

Re: [PATCH 1/3] Teach 'git pull' to handle --rebase=interactive

[Johannes Schindelin]

Hi Junio,

On Tue, 12 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Subject: Re: [PATCH 1/3] Teach 'git pull' to handle --rebase=interactive
> 
> Subject: pull: allow interactive rebase with --rebase=interactive
> 
> or something?  The same comment applies for the other patches in
> this series, and other recent patches from you in general.

Fixed in v2.

> > The way builtin pull works, this incidentally also supports the value
> > 'interactive' for the 'branch.<name>.rebase' config variable.
> 
> That, especially the "incidentally" part, makes it sound as if it is
> "because we can easily", not "because it is useful in such and such
> way, we must support it".  We definitely want to see patches that
> fall into the latter category, and from the sound of 0/3, I think
> this one should be advertised as such.

Also addressed in v2.

Thanks,
Dscho

[PATCH v2 0/3] Support `git pull --rebase=interactive`

[Johannes Schindelin]

A couple of years ago, I found the need to collaborate on topic branches
that were rebased all the time, and I really needed to see what I was
rebasing when pulling, so I introduced an interactively-rebasing pull.

This patch series ports that work to the builtin pull.

Apart from the change outlined in the interdiff, the first two commit
messages changed, too.

Johannes Schindelin (3):
  pull: allow interactive rebase with --rebase=interactive
  remote: handle the config setting branch.*.rebase=interactive
  completion: add missing branch.*.rebase values

 Documentation/config.txt               |  1 +
 Documentation/git-pull.txt             |  4 +++-
 builtin/pull.c                         |  7 ++++++-
 builtin/remote.c                       | 10 +++++++---
 contrib/completion/git-completion.bash |  2 +-
 t/t5520-pull.sh                        | 10 ++++++++++
 6 files changed, 28 insertions(+), 6 deletions(-)

Interdiff vs v1:

 diff --git a/builtin/remote.c b/builtin/remote.c
 index 0af8300..2b2ff9b 100644
 --- a/builtin/remote.c
 +++ b/builtin/remote.c
 @@ -311,7 +311,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)
  			if (v >= 0)
  				info->rebase = v;
  			else if (!strcmp(value, "preserve"))
 -				info->rebase = 1;
 +				info->rebase = NORMAL_REBASE;
  			else if (!strcmp(value, "interactive"))
  				info->rebase = INTERACTIVE_REBASE;
  		}

-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Johannes Schindelin]

The way builtin pull works, this change also supports the value
'interactive' for the 'branch.<name>.rebase' config variable, which
is a neat thing because users can now configure given branches for
interactively-rebasing pulls without having to type out the complete
`--rebase=interactive` option every time they pull.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt   |  1 +
 Documentation/git-pull.txt |  4 +++-
 builtin/pull.c             |  7 ++++++-
 t/t5520-pull.sh            | 10 ++++++++++
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 07f7a3b..e5897e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -865,6 +865,7 @@ branch.<name>.rebase::
 	instead of merging the default branch from the default remote when
 	"git pull" is run. See "pull.rebase" for doing this in a non
 	branch-specific manner.
+	When the value is `interactive`, the rebase is run in interactive mode.
 +
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 93c72a2..a62a2a6 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,7 +101,7 @@ Options related to merging
 include::merge-options.txt[]
 
 -r::
---rebase[=false|true|preserve]::
+--rebase[=false|true|preserve|interactive]::
 	When true, rebase the current branch on top of the upstream
 	branch after fetching. If there is a remote-tracking branch
 	corresponding to the upstream branch and the upstream branch
@@ -113,6 +113,8 @@ to `git rebase` so that locally created merge commits will not be flattened.
 +
 When false, merge the current branch into the upstream branch.
 +
+When `interactive`, enable the interactive mode of rebase.
++
 See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `--rebase` instead of merging.
diff --git a/builtin/pull.c b/builtin/pull.c
index 9e3c738..832d0ad 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -22,7 +22,8 @@ enum rebase_type {
 	REBASE_INVALID = -1,
 	REBASE_FALSE = 0,
 	REBASE_TRUE,
-	REBASE_PRESERVE
+	REBASE_PRESERVE,
+	REBASE_INTERACTIVE
 };
 
 /**
@@ -42,6 +43,8 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
 		return REBASE_TRUE;
 	else if (!strcmp(value, "preserve"))
 		return REBASE_PRESERVE;
+	else if (!strcmp(value, "interactive"))
+		return REBASE_INTERACTIVE;
 
 	if (fatal)
 		die(_("Invalid value for %s: %s"), key, value);
@@ -778,6 +781,8 @@ static int run_rebase(const unsigned char *curr_head,
 	/* Options passed to git-rebase */
 	if (opt_rebase == REBASE_PRESERVE)
 		argv_array_push(&args, "--preserve-merges");
+	else if (opt_rebase == REBASE_INTERACTIVE)
+		argv_array_push(&args, "--interactive");
 	if (opt_diffstat)
 		argv_array_push(&args, opt_diffstat);
 	argv_array_pushv(&args, opt_strategies.argv);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee..c952d5e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -326,6 +326,16 @@ test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
 	test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)"
 '
 
+test_expect_success 'pull.rebase=interactive' '
+	write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
+	echo I was here >fake.out &&
+	false
+	EOF
+	test_set_editor "$TRASH_DIRECTORY/fake-editor" &&
+	test_must_fail git pull --rebase=interactive . copy &&
+	test "I was here" = "$(cat fake.out)"
+'
+
 test_expect_success 'pull.rebase=invalid fails' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase invalid &&
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v2 2/3] remote: handle the config setting branch.*.rebase=interactive

[Johannes Schindelin]

The config variable branch.<branchname>.rebase is not only used by `git
pull`, but also by `git remote` when showing details about a remote.
Therefore, it needs to be taught to accept the newly-introduced
`interactive` value of said variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/remote.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 6694cf2..2b2ff9b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
 struct branch_info {
 	char *remote_name;
 	struct string_list merge;
-	int rebase;
+	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
 };
 
 static struct string_list branch_list;
@@ -311,7 +311,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 			if (v >= 0)
 				info->rebase = v;
 			else if (!strcmp(value, "preserve"))
-				info->rebase = 1;
+				info->rebase = NORMAL_REBASE;
+			else if (!strcmp(value, "interactive"))
+				info->rebase = INTERACTIVE_REBASE;
 		}
 	}
 	return 0;
@@ -980,7 +982,9 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)
 
 	printf("    %-*s ", show_info->width, item->string);
 	if (branch_info->rebase) {
-		printf_ln(_("rebases onto remote %s"), merge->items[0].string);
+		printf_ln(_(branch_info->rebase == INTERACTIVE_REBASE ?
+			"rebases interactively onto remote %s" :
+			"rebases onto remote %s"), merge->items[0].string);
 		return 0;
 	} else if (show_info->any_rebase) {
 		printf_ln(_(" merges with remote %s"), merge->items[0].string);
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v2 3/3] completion: add missing branch.*.rebase values

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ab4da7f..51f5223 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1809,7 +1809,7 @@ _git_config ()
 		return
 		;;
 	branch.*.rebase)
-		__gitcomp "false true"
+		__gitcomp "false true preserve interactive"
 		return
 		;;
 	remote.pushdefault)
-- 
2.6.3.windows.1.300.g1c25e49

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Matthieu Moy]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/remote.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 6694cf2..0af8300 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
>  struct branch_info {
>  	char *remote_name;
>  	struct string_list merge;
> -	int rebase;
> +	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
>  };
>  
>  static struct string_list branch_list;
> @@ -312,6 +312,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
>  				info->rebase = v;
>  			else if (!strcmp(value, "preserve"))
>  				info->rebase = 1;
> +			else if (!strcmp(value, "interactive"))
> +				info->rebase = INTERACTIVE_REBASE;

What happens if one has branch.*.rebase=interactive, and wants to make
an exception? Does

  git pull --rebase=true

cancel the 'interactive' part? I guess it is, but if so I think it
should be tested and documented.

Anyway, thanks for the patch.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Paul Tan]

Hi Dscho,

On Wed, Jan 13, 2016 at 2:57 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 07f7a3b..e5897e9 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -865,6 +865,7 @@ branch.<name>.rebase::
>         instead of merging the default branch from the default remote when
>         "git pull" is run. See "pull.rebase" for doing this in a non
>         branch-specific manner.
> +       When the value is `interactive`, the rebase is run in interactive mode.
>  +
>  When preserve, also pass `--preserve-merges` along to 'git rebase'
>  so that locally committed merge commits will not be flattened

I think this change needs to be repeated for the section on
"pull.rebase" as well.

> [...]
> --
> 2.6.3.windows.1.300.g1c25e49

Other than that, builtin/pull.c has the following option definition
that needs to be updated:

{ OPTION_CALLBACK, 'r', "rebase", &opt_rebase, "false|true|preserve",
N_("incorporate changes by rebasing rather than merging"),

The "false|true|preserve" needs to be updated to
"false|true|preserve|interactive", I think.

Regards,
Paul

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Johannes Schindelin]

Hi Matthieu,

On Wed, 13 Jan 2016, Matthieu Moy wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > diff --git a/builtin/remote.c b/builtin/remote.c
> > index 6694cf2..0af8300 100644
> > --- a/builtin/remote.c
> > +++ b/builtin/remote.c
> >  static struct string_list branch_list;
> > @@ -312,6 +312,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
> >  				info->rebase = v;
> >  			else if (!strcmp(value, "preserve"))
> >  				info->rebase = 1;
> > +			else if (!strcmp(value, "interactive"))
> > +				info->rebase = INTERACTIVE_REBASE;
> 
> What happens if one has branch.*.rebase=interactive, and wants to make
> an exception? Does
> 
>   git pull --rebase=true
> 
> cancel the 'interactive' part?

It is the same situation as before (just substitute a
branch.*.rebase=preserve setting): yes, the config is parsed first, then
the command line, so the command line wins.

> I guess it is, but if so I think it should be tested and documented.

Is this really necessary, given that the behavior has not changed from
before?

Ciao,
Dscho

Re: [PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Johannes Schindelin]

Hi Paul,

On Wed, 13 Jan 2016, Paul Tan wrote:

> On Wed, Jan 13, 2016 at 2:57 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 07f7a3b..e5897e9 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -865,6 +865,7 @@ branch.<name>.rebase::
> >         instead of merging the default branch from the default remote when
> >         "git pull" is run. See "pull.rebase" for doing this in a non
> >         branch-specific manner.
> > +       When the value is `interactive`, the rebase is run in interactive mode.
> >  +
> >  When preserve, also pass `--preserve-merges` along to 'git rebase'
> >  so that locally committed merge commits will not be flattened
> 
> I think this change needs to be repeated for the section on
> "pull.rebase" as well.
> 
> > [...]
> > --
> > 2.6.3.windows.1.300.g1c25e49
> 
> Other than that, builtin/pull.c has the following option definition
> that needs to be updated:
> 
> { OPTION_CALLBACK, 'r', "rebase", &opt_rebase, "false|true|preserve",
> N_("incorporate changes by rebasing rather than merging"),
> 
> The "false|true|preserve" needs to be updated to
> "false|true|preserve|interactive", I think.

Good points, both. Fixed in the upcoming v3.

Ciao,
Dscho

[PATCH v3 0/3] Support `git pull --rebase=interactive`

[Johannes Schindelin]

A couple of years ago, I found the need to collaborate on topic branches
that were rebased all the time, and I really needed to see what I was
rebasing when pulling, so I introduced an interactively-rebasing pull.

This patch series ports that work to the builtin pull.


Johannes Schindelin (3):
  pull: allow interactive rebase with --rebase=interactive
  remote: handle the config setting branch.*.rebase=interactive
  completion: add missing branch.*.rebase values

 Documentation/config.txt               |  4 ++++
 Documentation/git-pull.txt             |  4 +++-
 builtin/pull.c                         |  9 +++++++--
 builtin/remote.c                       | 10 +++++++---
 contrib/completion/git-completion.bash |  2 +-
 t/t5520-pull.sh                        | 10 ++++++++++
 6 files changed, 32 insertions(+), 7 deletions(-)

Interdiff vs v2:

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index e5897e9..0f710ca 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -865,12 +865,13 @@ branch.<name>.rebase::
  	instead of merging the default branch from the default remote when
  	"git pull" is run. See "pull.rebase" for doing this in a non
  	branch-specific manner.
 -	When the value is `interactive`, the rebase is run in interactive mode.
  +
  When preserve, also pass `--preserve-merges` along to 'git rebase'
  so that locally committed merge commits will not be flattened
  by running 'git pull'.
  +
 +When the value is `interactive`, the rebase is run in interactive mode.
 ++
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
  for details).
 @@ -2158,6 +2159,8 @@ When preserve, also pass `--preserve-merges` along to 'git rebase'
  so that locally committed merge commits will not be flattened
  by running 'git pull'.
  +
 +When the value is `interactive`, the rebase is run in interactive mode.
 ++
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
  for details).
 diff --git a/builtin/pull.c b/builtin/pull.c
 index 832d0ad..c713fe0 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -116,7 +116,7 @@ static struct option pull_options[] = {
  	/* Options passed to git-merge or git-rebase */
  	OPT_GROUP(N_("Options related to merging")),
  	{ OPTION_CALLBACK, 'r', "rebase", &opt_rebase,
 -	  "false|true|preserve",
 +	  "false|true|preserve|interactive",
  	  N_("incorporate changes by rebasing rather than merging"),
  	  PARSE_OPT_OPTARG, parse_opt_rebase },
  	OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,

-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v3 1/3] pull: allow interactive rebase with --rebase=interactive

[Johannes Schindelin]

The way builtin pull works, this change also supports the value
'interactive' for the 'branch.<name>.rebase' config variable, which
is a neat thing because users can now configure given branches for
interactively-rebasing pulls without having to type out the complete
`--rebase=interactive` option every time they pull.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt   |  4 ++++
 Documentation/git-pull.txt |  4 +++-
 builtin/pull.c             |  9 +++++++--
 t/t5520-pull.sh            | 10 ++++++++++
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 07f7a3b..0f710ca 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -870,6 +870,8 @@ When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
 +
+When the value is `interactive`, the rebase is run in interactive mode.
++
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
@@ -2157,6 +2159,8 @@ When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
 +
+When the value is `interactive`, the rebase is run in interactive mode.
++
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
 for details).
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 93c72a2..a62a2a6 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,7 +101,7 @@ Options related to merging
 include::merge-options.txt[]
 
 -r::
---rebase[=false|true|preserve]::
+--rebase[=false|true|preserve|interactive]::
 	When true, rebase the current branch on top of the upstream
 	branch after fetching. If there is a remote-tracking branch
 	corresponding to the upstream branch and the upstream branch
@@ -113,6 +113,8 @@ to `git rebase` so that locally created merge commits will not be flattened.
 +
 When false, merge the current branch into the upstream branch.
 +
+When `interactive`, enable the interactive mode of rebase.
++
 See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `--rebase` instead of merging.
diff --git a/builtin/pull.c b/builtin/pull.c
index 9e3c738..c713fe0 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -22,7 +22,8 @@ enum rebase_type {
 	REBASE_INVALID = -1,
 	REBASE_FALSE = 0,
 	REBASE_TRUE,
-	REBASE_PRESERVE
+	REBASE_PRESERVE,
+	REBASE_INTERACTIVE
 };
 
 /**
@@ -42,6 +43,8 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
 		return REBASE_TRUE;
 	else if (!strcmp(value, "preserve"))
 		return REBASE_PRESERVE;
+	else if (!strcmp(value, "interactive"))
+		return REBASE_INTERACTIVE;
 
 	if (fatal)
 		die(_("Invalid value for %s: %s"), key, value);
@@ -113,7 +116,7 @@ static struct option pull_options[] = {
 	/* Options passed to git-merge or git-rebase */
 	OPT_GROUP(N_("Options related to merging")),
 	{ OPTION_CALLBACK, 'r', "rebase", &opt_rebase,
-	  "false|true|preserve",
+	  "false|true|preserve|interactive",
 	  N_("incorporate changes by rebasing rather than merging"),
 	  PARSE_OPT_OPTARG, parse_opt_rebase },
 	OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
@@ -778,6 +781,8 @@ static int run_rebase(const unsigned char *curr_head,
 	/* Options passed to git-rebase */
 	if (opt_rebase == REBASE_PRESERVE)
 		argv_array_push(&args, "--preserve-merges");
+	else if (opt_rebase == REBASE_INTERACTIVE)
+		argv_array_push(&args, "--interactive");
 	if (opt_diffstat)
 		argv_array_push(&args, opt_diffstat);
 	argv_array_pushv(&args, opt_strategies.argv);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee..c952d5e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -326,6 +326,16 @@ test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
 	test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)"
 '
 
+test_expect_success 'pull.rebase=interactive' '
+	write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
+	echo I was here >fake.out &&
+	false
+	EOF
+	test_set_editor "$TRASH_DIRECTORY/fake-editor" &&
+	test_must_fail git pull --rebase=interactive . copy &&
+	test "I was here" = "$(cat fake.out)"
+'
+
 test_expect_success 'pull.rebase=invalid fails' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase invalid &&
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v3 2/3] remote: handle the config setting branch.*.rebase=interactive

[Johannes Schindelin]

The config variable branch.<branchname>.rebase is not only used by `git
pull`, but also by `git remote` when showing details about a remote.
Therefore, it needs to be taught to accept the newly-introduced
`interactive` value of said variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/remote.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 6694cf2..2b2ff9b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -251,7 +251,7 @@ static int add(int argc, const char **argv)
 struct branch_info {
 	char *remote_name;
 	struct string_list merge;
-	int rebase;
+	enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
 };
 
 static struct string_list branch_list;
@@ -311,7 +311,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 			if (v >= 0)
 				info->rebase = v;
 			else if (!strcmp(value, "preserve"))
-				info->rebase = 1;
+				info->rebase = NORMAL_REBASE;
+			else if (!strcmp(value, "interactive"))
+				info->rebase = INTERACTIVE_REBASE;
 		}
 	}
 	return 0;
@@ -980,7 +982,9 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)
 
 	printf("    %-*s ", show_info->width, item->string);
 	if (branch_info->rebase) {
-		printf_ln(_("rebases onto remote %s"), merge->items[0].string);
+		printf_ln(_(branch_info->rebase == INTERACTIVE_REBASE ?
+			"rebases interactively onto remote %s" :
+			"rebases onto remote %s"), merge->items[0].string);
 		return 0;
 	} else if (show_info->any_rebase) {
 		printf_ln(_(" merges with remote %s"), merge->items[0].string);
-- 
2.6.3.windows.1.300.g1c25e49

[PATCH v3 3/3] completion: add missing branch.*.rebase values

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ab4da7f..51f5223 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1809,7 +1809,7 @@ _git_config ()
 		return
 		;;
 	branch.*.rebase)
-		__gitcomp "false true"
+		__gitcomp "false true preserve interactive"
 		return
 		;;
 	remote.pushdefault)
-- 
2.6.3.windows.1.300.g1c25e49

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Matthieu Moy]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Matthieu,
>
> On Wed, 13 Jan 2016, Matthieu Moy wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>
>> What happens if one has branch.*.rebase=interactive, and wants to make
>> an exception? Does
>> 
>>   git pull --rebase=true
>> 
>> cancel the 'interactive' part?
>
> It is the same situation as before (just substitute a
> branch.*.rebase=preserve setting): yes, the config is parsed first, then
> the command line, so the command line wins.
>
>> I guess it is, but if so I think it should be tested and documented.
>
> Is this really necessary, given that the behavior has not changed from
> before?

Well, before your patch, branch.*.rebase=interactive was not possible.
It is not immediately clear to me that --rebase=true can mean "do the
pull with rebasing, but without going interactive", as "do pull with
rebase" and "rebase interactively" could be two independant things.
Reading the current doc does not help much: "When true, rebase the
current branch on top of the upstream branch after fetching" => it does
not say that "true" also specifies which kind of rebase is performed.

Actually, I was about to suggest having --rebase=non-interactive when I
realized it was already there with the syntax --rebase=true.

I can live without it, but I think a documentation improvement would
make things clearer.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] Teach 'git remote' that the config var branch.*.rebase can be 'interactive'

[Johannes Schindelin]

Hi Matthieu,

On Wed, 13 Jan 2016, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Wed, 13 Jan 2016, Matthieu Moy wrote:
> >
> >> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >>
> >> What happens if one has branch.*.rebase=interactive, and wants to
> >> make an exception? Does
> >> 
> >>   git pull --rebase=true
> >> 
> >> cancel the 'interactive' part?
> >
> > It is the same situation as before (just substitute a
> > branch.*.rebase=preserve setting): yes, the config is parsed first,
> > then the command line, so the command line wins.
> >
> >> I guess it is, but if so I think it should be tested and documented.
> >
> > Is this really necessary, given that the behavior has not changed from
> > before?
> 
> Well, before your patch, branch.*.rebase=interactive was not possible.

But branch.*.rebase=preserve was. And it is still different from
branch.*.rebase=true. And as per v2.7.0, those config settings can be
overridden via the command line.

Seeing as I did not change that behavior, I would find it a bit odd to
document that the command line can override the config setting.

> It is not immediately clear to me that --rebase=true can mean "do the
> pull with rebasing, but without going interactive", as "do pull with
> rebase" and "rebase interactively" could be two independant things.
> Reading the current doc does not help much: "When true, rebase the
> current branch on top of the upstream branch after fetching" => it does
> not say that "true" also specifies which kind of rebase is performed.

Again, I did not change that behavior, and the same confusion could arise
with the "preserve" setting.

But I do not really see the confusion, as `git rebase` (without -p and
without -i) means: "perform a rebase", and everybody assumes that it is a
non-interactive, non-merge-preserving rebase. Hence I would assume that
users know that a rebase without any further adjective refers to the
plain, non-interactive, non-merge-preserving one.

Ciao,
Dscho

Re: [PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The way builtin pull works, this change also supports the value
> 'interactive' for the 'branch.<name>.rebase' config variable, which
> is a neat thing because users can now configure given branches for
> interactively-rebasing pulls without having to type out the complete
> `--rebase=interactive` option every time they pull.

Surely this is a better description about the "config also works and
it is a good thing" part.  What I was alluding to was actually this
you wrote in 0/3:

    A couple of years ago, I found the need to collaborate on topic
    branches that were rebased all the time, and I really needed to
    see what I was rebasing when pulling, so I introduced an
    interactively-rebasing pull.

that justifies why allowing 'pull' to invoke interactive rebase is a
good thing, which should go before all of the above five lines.

I see you have v3 already (and I read Paul's "you missed these"),
which I haven't read yet, but I expect the actual changes are good
and no longer need rerolling, so I may just insert these four lines
at the beginning of the log message while queuing, or something.

Thanks, both.

Re: [PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
>     A couple of years ago, I found the need to collaborate on topic
>     branches that were rebased all the time, and I really needed to see
>     what I was rebasing when pulling, so I introduced an
>     interactively-rebasing pull.
> 
> [...] I may just insert these four lines at the beginning of the log
> message while queuing, or something.

Oh, that's what you meant! Sorry for misunderstanding, and thanks for not
requiring a reroll.

Ciao,
Dscho

Re: [PATCH v2 1/3] pull: allow interactive rebase with --rebase=interactive

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Wed, 13 Jan 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> 
>>     A couple of years ago, I found the need to collaborate on topic
>>     branches that were rebased all the time, and I really needed to see
>>     what I was rebasing when pulling, so I introduced an
>>     interactively-rebasing pull.
>> 
>> [...] I may just insert these four lines at the beginning of the log
>> message while queuing, or something.
>
> Oh, that's what you meant! Sorry for misunderstanding, and thanks for not
> requiring a reroll.

Heh, that is too early, as I haven't even read v3 yet.  There we may
find other reasons that we would want to improve it before queuing.

Re: [PATCH v3 0/3] Support `git pull --rebase=interactive`

[Junio C Hamano]

Thanks, queued (with the promised amend to the proposed log message
of 1/3).