Re: [PATCH 04/11] builtin rebase: support --quiet
Hi Junio, On Wed, 8 Aug 2018, Junio C Hamano wrote: > Stefan Beller writes: > > > On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki wrote: > >> > >> This commit introduces a rebase option `--quiet`. While `--quiet` is > >> commonly perceived as opposite to `--verbose`, this is not the case for > >> the rebase command: both `--quiet` and `--verbose` default to `false` if > >> neither `--quiet` nor `--verbose` is present. > >> > >> This commit goes further and introduces `--no-quiet` which is the > >> contrary of `--quiet` and it's introduction doesn't modify any > >> behaviour. > > Why? Is it for completeness (i.e. does the scripted version take > such an option and addition of --no-quiet makes the C rewrite behave > the same)? Ah. I mentioned that an explanation for this is needed in the commit message, and I guess that it is a bit too subtle. The part you clipped from your quoted text says: [... `--quiet`] switches off --verbose and --stat, and as --verbose switches off --quiet, we use the (negated) REBASE_NO_QUIET instead of REBASE_QUIET: this allows us to turn off the quiet mode and turn on the verbose and diffstat mode in a single OPT_BIT(), and the opposite in a single OPT_NEGBIT(). I agree that this is a pretty convoluted way to express the issue. See below for an attempt at a clearer commit message. > >> Note: The `flags` field in `rebase_options` will accumulate more bits in > >> subsequent commits, in particular a verbose and a diffstat flag. And as > >> --quoet inthe shell scripted version of the rebase command switches off > > > > --quote in the > > > > (in case a resend is needed) > > Meaning --quiet? Yep. I should have paid more attention in my pre-submission review, sorry. I changed the commit message to read like this: builtin rebase: support --quiet This commit introduces a rebase option `--quiet`. While `--quiet` is commonly perceived as opposite to `--verbose`, this is not the case for the rebase command: both `--quiet` and `--verbose` default to `false` if neither `--quiet` nor `--verbose` is present. Despite the default being `false` for both verbose and quiet mode, passing the `--quiet` option will turn off verbose mode, and `--verbose` will turn off quiet mode. This patch introduces the `flags` bit field, with `REBASE_NO_QUIET` as first user (with many more to come). We do *not* use `REBASE_QUIET` here for an important reason: To keep the implementation simple, this commit introduces `--no-quiet` instead of `--quiet`, so that a single `OPT_NEGBIT()` can turn on quiet mode and turn off verbose and diffstat mode at the same time. Likewise, the companion commit which will introduce support for `--verbose` will have a single `OPT_BIT()` that turns off quiet mode and turns on verbose and diffstat mode at the same time. Ciao, Dscho
Re: [PATCH 04/11] builtin rebase: support --quiet
Stefan Beller writes: > On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki wrote: >> >> This commit introduces a rebase option `--quiet`. While `--quiet` is >> commonly perceived as opposite to `--verbose`, this is not the case for >> the rebase command: both `--quiet` and `--verbose` default to `false` if >> neither `--quiet` nor `--verbose` is present. >> >> This commit goes further and introduces `--no-quiet` which is the >> contrary of `--quiet` and it's introduction doesn't modify any >> behaviour. Why? Is it for completeness (i.e. does the scripted version take such an option and addition of --no-quiet makes the C rewrite behave the same)? >> Note: The `flags` field in `rebase_options` will accumulate more bits in >> subsequent commits, in particular a verbose and a diffstat flag. And as >> --quoet inthe shell scripted version of the rebase command switches off > > --quote in the > > (in case a resend is needed) Meaning --quiet?
Re: [PATCH 04/11] builtin rebase: support --quiet
On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki wrote: > > This commit introduces a rebase option `--quiet`. While `--quiet` is > commonly perceived as opposite to `--verbose`, this is not the case for > the rebase command: both `--quiet` and `--verbose` default to `false` if > neither `--quiet` nor `--verbose` is present. > > This commit goes further and introduces `--no-quiet` which is the > contrary of `--quiet` and it's introduction doesn't modify any > behaviour. > > Note: The `flags` field in `rebase_options` will accumulate more bits in > subsequent commits, in particular a verbose and a diffstat flag. And as > --quoet inthe shell scripted version of the rebase command switches off --quote in the (in case a resend is needed)
[PATCH 04/11] builtin rebase: support --quiet
This commit introduces a rebase option `--quiet`. While `--quiet` is
commonly perceived as opposite to `--verbose`, this is not the case for
the rebase command: both `--quiet` and `--verbose` default to `false` if
neither `--quiet` nor `--verbose` is present.
This commit goes further and introduces `--no-quiet` which is the
contrary of `--quiet` and it's introduction doesn't modify any
behaviour.
Note: The `flags` field in `rebase_options` will accumulate more bits in
subsequent commits, in particular a verbose and a diffstat flag. And as
--quoet inthe shell scripted version of the rebase command switches off
--verbose and --stat, and as --verbose switches off --quiet, we use the
(negated) REBASE_NO_QUIET instead of REBASE_QUIET: this allows us to
turn off the quiet mode and turn on the verbose and diffstat mode in a
single OPT_BIT(), and the opposite in a single OPT_NEGBIT().
Signed-off-by: Pratik Karki
---
builtin/rebase.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index b79f9b0a9f..19fa4d3fc4 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -79,6 +79,10 @@ struct rebase_options {
int root;
struct commit *restrict_revision;
int dont_finish_rebase;
+ enum {
+ REBASE_NO_QUIET = 1<<0,
+ } flags;
+ struct strbuf git_am_opt;
};
/* Returns the filename prefixed by the state_dir */
@@ -159,6 +163,9 @@ static int run_specific_rebase(struct rebase_options *opts)
add_var(_snippet, "revisions", opts->revisions);
add_var(_snippet, "restrict_revision", opts->restrict_revision ?
oid_to_hex(>restrict_revision->object.oid) : NULL);
+ add_var(_snippet, "GIT_QUIET",
+ opts->flags & REBASE_NO_QUIET ? "" : "t");
+ add_var(_snippet, "git_am_opt", opts->git_am_opt.buf);
switch (opts->type) {
case REBASE_AM:
@@ -308,6 +315,8 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
{
struct rebase_options options = {
.type = REBASE_UNSPECIFIED,
+ .flags = REBASE_NO_QUIET,
+ .git_am_opt = STRBUF_INIT,
};
const char *branch_name;
int ret, flags;
@@ -321,6 +330,9 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
N_("rebase onto given branch instead of upstream")),
OPT_BOOL(0, "no-verify", _to_skip_pre_rebase,
N_("allow pre-rebase hook to run")),
+ OPT_NEGBIT('q', "quiet", ,
+ N_("be quiet. implies --no-stat"),
+ REBASE_NO_QUIET),
OPT_END(),
};
@@ -357,6 +369,9 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
usage_with_options(builtin_rebase_usage,
builtin_rebase_options);
+ if (!(options.flags & REBASE_NO_QUIET))
+ strbuf_addstr(_am_opt, " -q");
+
switch (options.type) {
case REBASE_MERGE:
case REBASE_INTERACTIVE:
--
2.18.0
