| Message ID | 20181211161139.31686-6-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reimplement rebase --merge via interactive machinery | expand |
Hi Elijah, On Tue, 11 Dec 2018, Elijah Newren wrote: > While 'quiet' and 'interactive' may sound like antonyms, the interactive > machinery actually has logic that implements several > interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) > which won't pop up an editor. The rewrite of interactive rebase in C > added a quiet option, though it only turns stats off. Since we want to > make the interactive machinery also take over for git-rebase--merge, it > should fully implement the --quiet option. > > git-rebase--interactive was already somewhat quieter than > git-rebase--merge and git-rebase--am, possibly because cherry-pick has > just traditionally been quieter. As such, we only drop a few > informational messages -- "Rebasing (n/m)" and "Successfully rebased..." > > Also, for simplicity, remove the differences in how quiet and verbose > options were recorded. Having one be signalled by the presence of a > "verbose" file in the state_dir, while the other was signalled by the > contents of a "quiet" file was just weirdly inconsistent. (This > inconsistency pre-dated the rewrite into C.) Make them consistent by > having them both key off the presence of the file. > > Signed-off-by: Elijah Newren <newren@gmail.com> This is convincing. I would like to point out, though... > --- > builtin/rebase.c | 5 +---- > git-legacy-rebase.sh | 2 +- > git-rebase--common.sh | 2 +- > sequencer.c | 23 +++++++++++++---------- > sequencer.h | 1 + > 5 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 78e982298f..ec2e5fbf23 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -185,10 +185,7 @@ static int read_basic_state(struct rebase_options *opts) > if (get_oid(buf.buf, &opts->orig_head)) > return error(_("invalid orig-head: '%s'"), buf.buf); > > - strbuf_reset(&buf); > - if (read_one(state_dir_path("quiet", opts), &buf)) > - return -1; > - if (buf.len) > + if (file_exists(state_dir_path("quiet", opts))) This changes the behavior. AFAIR the `quiet` file was always written, but contained `t` in quiet mode. I have to interrupt my review here, and will continue later, but maybe you will beat me to looking into that. Ciao, Dscho > opts->flags &= ~REBASE_NO_QUIET; > else > opts->flags |= REBASE_NO_QUIET; > diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh > index fccb33b959..f4088b7bda 100755 > --- a/git-legacy-rebase.sh > +++ b/git-legacy-rebase.sh > @@ -113,7 +113,7 @@ read_basic_state () { > else > orig_head=$(cat "$state_dir"/head) > fi && > - GIT_QUIET=$(cat "$state_dir"/quiet) && > + test -f "$state_dir"/quiet && GIT_QUIET=t > test -f "$state_dir"/verbose && verbose=t > test -f "$state_dir"/strategy && strategy="$(cat "$state_dir"/strategy)" > test -f "$state_dir"/strategy_opts && > diff --git a/git-rebase--common.sh b/git-rebase--common.sh > index 7e39d22871..dc18c682fa 100644 > --- a/git-rebase--common.sh > +++ b/git-rebase--common.sh > @@ -10,7 +10,7 @@ write_basic_state () { > echo "$head_name" > "$state_dir"/head-name && > echo "$onto" > "$state_dir"/onto && > echo "$orig_head" > "$state_dir"/orig-head && > - echo "$GIT_QUIET" > "$state_dir"/quiet && > + test t = "$GIT_QUIET" && : > "$state_dir"/quiet > test t = "$verbose" && : > "$state_dir"/verbose > test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy > test -n "$strategy_opts" && echo "$strategy_opts" > \ > diff --git a/sequencer.c b/sequencer.c > index e1a4dd15f1..bc25615050 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -150,6 +150,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") > static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") > static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") > static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") > +static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") > static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff") > static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name") > static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto") > @@ -157,7 +158,6 @@ static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash") > static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy") > static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts") > static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate") > -static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") > > static int git_sequencer_config(const char *k, const char *v, void *cb) > { > @@ -2357,6 +2357,9 @@ static int read_populate_opts(struct replay_opts *opts) > if (file_exists(rebase_path_verbose())) > opts->verbose = 1; > > + if (file_exists(rebase_path_quiet())) > + opts->quiet = 1; > + > if (file_exists(rebase_path_signoff())) { > opts->allow_ff = 0; > opts->signoff = 1; > @@ -2424,9 +2427,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, > > if (quiet) > write_file(rebase_path_quiet(), "%s\n", quiet); > - else > - write_file(rebase_path_quiet(), "\n"); > - > if (opts->verbose) > write_file(rebase_path_verbose(), "%s", ""); > if (opts->strategy) > @@ -3503,10 +3503,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) > fprintf(f, "%d\n", todo_list->done_nr); > fclose(f); > } > - fprintf(stderr, "Rebasing (%d/%d)%s", > - todo_list->done_nr, > - todo_list->total_nr, > - opts->verbose ? "\n" : "\r"); > + if (!opts->quiet) > + fprintf(stderr, "Rebasing (%d/%d)%s", > + todo_list->done_nr, > + todo_list->total_nr, > + opts->verbose ? "\n" : "\r"); > } > unlink(rebase_path_message()); > unlink(rebase_path_author_script()); > @@ -3738,8 +3739,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) > } > apply_autostash(opts); > > - fprintf(stderr, "Successfully rebased and updated %s.\n", > - head_ref.buf); > + if (!opts->quiet) > + fprintf(stderr, > + "Successfully rebased and updated %s.\n", > + head_ref.buf); > > strbuf_release(&buf); > strbuf_release(&head_ref); > diff --git a/sequencer.h b/sequencer.h > index 5071a73563..729222b583 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -39,6 +39,7 @@ struct replay_opts { > int allow_empty_message; > int keep_redundant_commits; > int verbose; > + int quiet; > > int mainline; > > -- > 2.20.0.8.g5de428d695 > >
Hi Dscho, On Mon, Jan 21, 2019 at 8:10 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Elijah, > > On Tue, 11 Dec 2018, Elijah Newren wrote: > > > While 'quiet' and 'interactive' may sound like antonyms, the interactive > > machinery actually has logic that implements several > > interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) > > which won't pop up an editor. The rewrite of interactive rebase in C > > added a quiet option, though it only turns stats off. Since we want to > > make the interactive machinery also take over for git-rebase--merge, it > > should fully implement the --quiet option. > > > > git-rebase--interactive was already somewhat quieter than > > git-rebase--merge and git-rebase--am, possibly because cherry-pick has > > just traditionally been quieter. As such, we only drop a few > > informational messages -- "Rebasing (n/m)" and "Successfully rebased..." > > > > Also, for simplicity, remove the differences in how quiet and verbose > > options were recorded. Having one be signalled by the presence of a > > "verbose" file in the state_dir, while the other was signalled by the > > contents of a "quiet" file was just weirdly inconsistent. (This > > inconsistency pre-dated the rewrite into C.) Make them consistent by > > having them both key off the presence of the file. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > This is convincing. I would like to point out, though... > > > --- > > builtin/rebase.c | 5 +---- > > git-legacy-rebase.sh | 2 +- > > git-rebase--common.sh | 2 +- > > sequencer.c | 23 +++++++++++++---------- > > sequencer.h | 1 + > > 5 files changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/builtin/rebase.c b/builtin/rebase.c > > index 78e982298f..ec2e5fbf23 100644 > > --- a/builtin/rebase.c > > +++ b/builtin/rebase.c > > @@ -185,10 +185,7 @@ static int read_basic_state(struct rebase_options *opts) > > if (get_oid(buf.buf, &opts->orig_head)) > > return error(_("invalid orig-head: '%s'"), buf.buf); > > > > - strbuf_reset(&buf); > > - if (read_one(state_dir_path("quiet", opts), &buf)) > > - return -1; > > - if (buf.len) > > + if (file_exists(state_dir_path("quiet", opts))) > > This changes the behavior. AFAIR the `quiet` file was always written, but > contained `t` in quiet mode. I have to interrupt my review here, and will > continue later, but maybe you will beat me to looking into that. > > Ciao, > Dscho You are certainly consistent; you commented on this exact same issue in both v1 and v2 (you didn't have time to review v3). In v2, your comment was[1]: " I am slightly concerned that some creative power user could have written scripts that rely on this behavior. But only *slightly* concerned. The patch looks correct. " Also, I have a fuzzy memory of discussing a very similar case with some rebase-oriented option and its on-disk representation, where the concern was more about users upgrading git versions during an incomplete rebase rather than power users looking at internal file contents. And I think either Phillip or Junio made some statement about considering these internal details and that they felt the worry about upgrade mid-rebase was overly worrying. But I can't find the emails right now, and it's been so long (at least half a year) that I might be imagining things. But from first principles, in this case even if you're worried about power users reaching into internal files of rebase or about users upgrade mid-rebase, at *worst* people will get or lose a few fluffy progress-update messages. To me, that's an innocuous level of change that's certainly worth the risk to allow us to get rid of the annoying differences in implementation of handling of different options. But, if you strongly feel that's too big a risk, we can remove the part of the patch making verbose and quiet be handled consistently; it's not critical to the rest of the series. I just thought it was a good cleanup while I was touching the area. Elijah [1] https://public-inbox.org/git/nycvar.QRO.7.76.6.1811121610350.39@tvgsbejvaqbjf.bet/
Hi Elijah, On Mon, 21 Jan 2019, Elijah Newren wrote: > On Mon, Jan 21, 2019 at 8:10 AM Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > > On Tue, 11 Dec 2018, Elijah Newren wrote: > > > > > While 'quiet' and 'interactive' may sound like antonyms, the interactive > > > machinery actually has logic that implements several > > > interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) > > > which won't pop up an editor. The rewrite of interactive rebase in C > > > added a quiet option, though it only turns stats off. Since we want to > > > make the interactive machinery also take over for git-rebase--merge, it > > > should fully implement the --quiet option. > > > > > > git-rebase--interactive was already somewhat quieter than > > > git-rebase--merge and git-rebase--am, possibly because cherry-pick has > > > just traditionally been quieter. As such, we only drop a few > > > informational messages -- "Rebasing (n/m)" and "Successfully rebased..." > > > > > > Also, for simplicity, remove the differences in how quiet and verbose > > > options were recorded. Having one be signalled by the presence of a > > > "verbose" file in the state_dir, while the other was signalled by the > > > contents of a "quiet" file was just weirdly inconsistent. (This > > > inconsistency pre-dated the rewrite into C.) Make them consistent by > > > having them both key off the presence of the file. > > > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > > > This is convincing. I would like to point out, though... > > > > > --- > > > builtin/rebase.c | 5 +---- > > > git-legacy-rebase.sh | 2 +- > > > git-rebase--common.sh | 2 +- > > > sequencer.c | 23 +++++++++++++---------- > > > sequencer.h | 1 + > > > 5 files changed, 17 insertions(+), 16 deletions(-) > > > > > > diff --git a/builtin/rebase.c b/builtin/rebase.c > > > index 78e982298f..ec2e5fbf23 100644 > > > --- a/builtin/rebase.c > > > +++ b/builtin/rebase.c > > > @@ -185,10 +185,7 @@ static int read_basic_state(struct rebase_options *opts) > > > if (get_oid(buf.buf, &opts->orig_head)) > > > return error(_("invalid orig-head: '%s'"), buf.buf); > > > > > > - strbuf_reset(&buf); > > > - if (read_one(state_dir_path("quiet", opts), &buf)) > > > - return -1; > > > - if (buf.len) > > > + if (file_exists(state_dir_path("quiet", opts))) > > > > This changes the behavior. AFAIR the `quiet` file was always written, but > > contained `t` in quiet mode. I have to interrupt my review here, and will > > continue later, but maybe you will beat me to looking into that. > > > > Ciao, > > Dscho > > You are certainly consistent; You flatter me! > you commented on this exact same issue in both v1 and v2 (you didn't > have time to review v3). In v2, your comment was[1]: > > " > I am slightly concerned that some creative power user could have written > scripts that rely on this behavior. > > But only *slightly* concerned. > > The patch looks correct. > " > > Also, I have a fuzzy memory of discussing a very similar case with > some rebase-oriented option and its on-disk representation, where the > concern was more about users upgrading git versions during an > incomplete rebase rather than power users looking at internal file > contents. And I think either Phillip or Junio made some statement > about considering these internal details and that they felt the worry > about upgrade mid-rebase was overly worrying. But I can't find the > emails right now, and it's been so long (at least half a year) that I > might be imagining things. Indeed, it was one of Phillip's patch series that fixed an incorrect quoting in the author script. And we eventually all agreed that it would not be worth the trouble to take care of upgrade-spanning rebases (both Phillip and I spent some time to make it so, IIRC). > But from first principles, in this case even if you're worried about > power users reaching into internal files of rebase or about users > upgrade mid-rebase, at *worst* people will get or lose a few fluffy > progress-update messages. To me, that's an innocuous level of change > that's certainly worth the risk to allow us to get rid of the annoying > differences in implementation of handling of different options. But, > if you strongly feel that's too big a risk, we can remove the part of > the patch making verbose and quiet be handled consistently; it's not > critical to the rest of the series. I just thought it was a good > cleanup while I was touching the area. I think you are correct. It is much easier in C to check whether a file exists than to read it, parse it and determine whether its Boolean value should be `false` or `true`. So color me convinced that your clean-up is worth it. Maybe, just maybe, if you submit another iteration (and only if), could you add a little paragraph for future Dscho to understand that this clean-up was slipped in? Thanks, Dscho
Hi Elijah, On Mon, 21 Jan 2019, Johannes Schindelin wrote: > Maybe, just maybe, if you submit another iteration (and only if), could > you add a little paragraph for future Dscho to understand that this > clean-up was slipped in? Oh wow. I guess I should learn how to read. The last paragraph says exactly that. For some reason, I missed that earlier. Sorry for the noise, Dscho
Elijah Newren <newren@gmail.com> writes: > Also, I have a fuzzy memory of discussing a very similar case with > some rebase-oriented option and its on-disk representation, where the > concern was more about users upgrading git versions during an > incomplete rebase rather than power users looking at internal file > contents. And I think either Phillip or Junio made some statement > about considering these internal details and that they felt the worry > about upgrade mid-rebase was overly worrying. But I can't find the > emails right now, and it's been so long (at least half a year) that I > might be imagining things. I do recall saying that mid-rebase upgrade is probably not worth getting worried about.
On 22/01/2019 20:39, Junio C Hamano wrote: > Elijah Newren <newren@gmail.com> writes: > >> Also, I have a fuzzy memory of discussing a very similar case with >> some rebase-oriented option and its on-disk representation, where the >> concern was more about users upgrading git versions during an >> incomplete rebase rather than power users looking at internal file >> contents. And I think either Phillip or Junio made some statement >> about considering these internal details and that they felt the worry >> about upgrade mid-rebase was overly worrying. But I can't find the >> emails right now, and it's been so long (at least half a year) that I >> might be imagining things. > > I do recall saying that mid-rebase upgrade is probably not worth > getting worried about. > In light of yesterday's bug report [1] about those other changes I'm more concerned about this change. We were worrying about whether or not to worry about a mid-rebase upgrade but it seems people can have two different versions of git installed - one bundled with something like tig and another they use on the command line. If they start a rebase with a version containing this patch and try to continue it with a version that does not then the older version will fail with a complaint about a missing quiet file. The other way round they'll potentially get the wrong quiet setting which isn't such a problem. It's probably a bit late in the release cycle now to change this? But we could flag it up in the release notes and bear it in mind when making changes in the future. Best Wishes Phillip [1] https://public-inbox.org/git/d7acf780-522d-84e6-68b8-d8d35a305588@talktalk.net/T/#maa521f788bd61f9b65d52c14430a88bf077e6752
Hi Phillip, On Wed, Feb 20, 2019 at 3:00 AM Phillip Wood <phillip.wood@talktalk.net> wrote: > On 22/01/2019 20:39, Junio C Hamano wrote: > > Elijah Newren <newren@gmail.com> writes: > > > >> Also, I have a fuzzy memory of discussing a very similar case with > >> some rebase-oriented option and its on-disk representation, where the > >> concern was more about users upgrading git versions during an > >> incomplete rebase rather than power users looking at internal file > >> contents. And I think either Phillip or Junio made some statement > >> about considering these internal details and that they felt the worry > >> about upgrade mid-rebase was overly worrying. But I can't find the > >> emails right now, and it's been so long (at least half a year) that I > >> might be imagining things. > > > > I do recall saying that mid-rebase upgrade is probably not worth > > getting worried about. > > > > In light of yesterday's bug report [1] about those other changes I'm > more concerned about this change. We were worrying about whether or not > to worry about a mid-rebase upgrade but it seems people can have two > different versions of git installed - one bundled with something like > tig and another they use on the command line. If they start a rebase > with a version containing this patch and try to continue it with a > version that does not then the older version will fail with a complaint > about a missing quiet file. The other way round they'll potentially get > the wrong quiet setting which isn't such a problem. It's probably a bit > late in the release cycle now to change this? But we could flag it up in > the release notes and bear it in mind when making changes in the future. Thanks for the heads up; I'll try to keep it in mind for the future.
diff --git a/builtin/rebase.c b/builtin/rebase.c index 78e982298f..ec2e5fbf23 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -185,10 +185,7 @@ static int read_basic_state(struct rebase_options *opts) if (get_oid(buf.buf, &opts->orig_head)) return error(_("invalid orig-head: '%s'"), buf.buf); - strbuf_reset(&buf); - if (read_one(state_dir_path("quiet", opts), &buf)) - return -1; - if (buf.len) + if (file_exists(state_dir_path("quiet", opts))) opts->flags &= ~REBASE_NO_QUIET; else opts->flags |= REBASE_NO_QUIET; diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh index fccb33b959..f4088b7bda 100755 --- a/git-legacy-rebase.sh +++ b/git-legacy-rebase.sh @@ -113,7 +113,7 @@ read_basic_state () { else orig_head=$(cat "$state_dir"/head) fi && - GIT_QUIET=$(cat "$state_dir"/quiet) && + test -f "$state_dir"/quiet && GIT_QUIET=t test -f "$state_dir"/verbose && verbose=t test -f "$state_dir"/strategy && strategy="$(cat "$state_dir"/strategy)" test -f "$state_dir"/strategy_opts && diff --git a/git-rebase--common.sh b/git-rebase--common.sh index 7e39d22871..dc18c682fa 100644 --- a/git-rebase--common.sh +++ b/git-rebase--common.sh @@ -10,7 +10,7 @@ write_basic_state () { echo "$head_name" > "$state_dir"/head-name && echo "$onto" > "$state_dir"/onto && echo "$orig_head" > "$state_dir"/orig-head && - echo "$GIT_QUIET" > "$state_dir"/quiet && + test t = "$GIT_QUIET" && : > "$state_dir"/quiet test t = "$verbose" && : > "$state_dir"/verbose test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy test -n "$strategy_opts" && echo "$strategy_opts" > \ diff --git a/sequencer.c b/sequencer.c index e1a4dd15f1..bc25615050 100644 --- a/sequencer.c +++ b/sequencer.c @@ -150,6 +150,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") +static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff") static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name") static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto") @@ -157,7 +158,6 @@ static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash") static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy") static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts") static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate") -static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") static int git_sequencer_config(const char *k, const char *v, void *cb) { @@ -2357,6 +2357,9 @@ static int read_populate_opts(struct replay_opts *opts) if (file_exists(rebase_path_verbose())) opts->verbose = 1; + if (file_exists(rebase_path_quiet())) + opts->quiet = 1; + if (file_exists(rebase_path_signoff())) { opts->allow_ff = 0; opts->signoff = 1; @@ -2424,9 +2427,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, if (quiet) write_file(rebase_path_quiet(), "%s\n", quiet); - else - write_file(rebase_path_quiet(), "\n"); - if (opts->verbose) write_file(rebase_path_verbose(), "%s", ""); if (opts->strategy) @@ -3503,10 +3503,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) fprintf(f, "%d\n", todo_list->done_nr); fclose(f); } - fprintf(stderr, "Rebasing (%d/%d)%s", - todo_list->done_nr, - todo_list->total_nr, - opts->verbose ? "\n" : "\r"); + if (!opts->quiet) + fprintf(stderr, "Rebasing (%d/%d)%s", + todo_list->done_nr, + todo_list->total_nr, + opts->verbose ? "\n" : "\r"); } unlink(rebase_path_message()); unlink(rebase_path_author_script()); @@ -3738,8 +3739,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) } apply_autostash(opts); - fprintf(stderr, "Successfully rebased and updated %s.\n", - head_ref.buf); + if (!opts->quiet) + fprintf(stderr, + "Successfully rebased and updated %s.\n", + head_ref.buf); strbuf_release(&buf); strbuf_release(&head_ref); diff --git a/sequencer.h b/sequencer.h index 5071a73563..729222b583 100644 --- a/sequencer.h +++ b/sequencer.h @@ -39,6 +39,7 @@ struct replay_opts { int allow_empty_message; int keep_redundant_commits; int verbose; + int quiet; int mainline;
While 'quiet' and 'interactive' may sound like antonyms, the interactive machinery actually has logic that implements several interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) which won't pop up an editor. The rewrite of interactive rebase in C added a quiet option, though it only turns stats off. Since we want to make the interactive machinery also take over for git-rebase--merge, it should fully implement the --quiet option. git-rebase--interactive was already somewhat quieter than git-rebase--merge and git-rebase--am, possibly because cherry-pick has just traditionally been quieter. As such, we only drop a few informational messages -- "Rebasing (n/m)" and "Successfully rebased..." Also, for simplicity, remove the differences in how quiet and verbose options were recorded. Having one be signalled by the presence of a "verbose" file in the state_dir, while the other was signalled by the contents of a "quiet" file was just weirdly inconsistent. (This inconsistency pre-dated the rewrite into C.) Make them consistent by having them both key off the presence of the file. Signed-off-by: Elijah Newren <newren@gmail.com> --- builtin/rebase.c | 5 +---- git-legacy-rebase.sh | 2 +- git-rebase--common.sh | 2 +- sequencer.c | 23 +++++++++++++---------- sequencer.h | 1 + 5 files changed, 17 insertions(+), 16 deletions(-)