From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: bmwill@google•com, novalis@novalis•org, git@vger•kernel.org
Subject: Re: [PATCH 1/4] read-tree: use OPT_BOOL instead of OPT_SET_INT
Date: Tue, 10 Jan 2017 12:41:31 -0800 [thread overview]
Message-ID: <xmqqmveyr6kk.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170110014542.19352-2-sbeller@google.com> (Stefan Beller's message of "Mon, 9 Jan 2017 17:45:39 -0800")
Stefan Beller <sbeller@google•com> writes:
> All occurrences of OPT_SET_INT were setting the value to 1;
> internally OPT_BOOL is just that.
>
> Signed-off-by: Stefan Beller <sbeller@google•com>
> ---
> builtin/read-tree.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
The result is much easier to read, partly because the "1" (true) is
at the very end of the line when OPT_SET_INT() is used for the
reader to notice that this is merely a boolean.
More importantly, as OPT_SET_INT() can be used multiple times on the
same variable (or field) to set it to different values, it is easier
to read if OPT_BOOL() is used when OPT_SET_INT() to 1 is not used
for that purpose.
Thanks.
>> diff --git a/builtin/read-tree.c b/builtin/read-tree.c
> index fa6edb35b2..8ba64bc785 100644
> --- a/builtin/read-tree.c
> +++ b/builtin/read-tree.c
> @@ -109,34 +109,34 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
> { OPTION_CALLBACK, 0, "index-output", NULL, N_("file"),
> N_("write resulting index to <file>"),
> PARSE_OPT_NONEG, index_output_cb },
> - OPT_SET_INT(0, "empty", &read_empty,
> - N_("only empty the index"), 1),
> + OPT_BOOL(0, "empty", &read_empty,
> + N_("only empty the index")),
> OPT__VERBOSE(&opts.verbose_update, N_("be verbose")),
> OPT_GROUP(N_("Merging")),
> - OPT_SET_INT('m', NULL, &opts.merge,
> - N_("perform a merge in addition to a read"), 1),
> - OPT_SET_INT(0, "trivial", &opts.trivial_merges_only,
> - N_("3-way merge if no file level merging required"), 1),
> - OPT_SET_INT(0, "aggressive", &opts.aggressive,
> - N_("3-way merge in presence of adds and removes"), 1),
> - OPT_SET_INT(0, "reset", &opts.reset,
> - N_("same as -m, but discard unmerged entries"), 1),
> + OPT_BOOL('m', NULL, &opts.merge,
> + N_("perform a merge in addition to a read")),
> + OPT_BOOL(0, "trivial", &opts.trivial_merges_only,
> + N_("3-way merge if no file level merging required")),
> + OPT_BOOL(0, "aggressive", &opts.aggressive,
> + N_("3-way merge in presence of adds and removes")),
> + OPT_BOOL(0, "reset", &opts.reset,
> + N_("same as -m, but discard unmerged entries")),
> { OPTION_STRING, 0, "prefix", &opts.prefix, N_("<subdirectory>/"),
> N_("read the tree into the index under <subdirectory>/"),
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },
> - OPT_SET_INT('u', NULL, &opts.update,
> - N_("update working tree with merge result"), 1),
> + OPT_BOOL('u', NULL, &opts.update,
> + N_("update working tree with merge result")),
> { OPTION_CALLBACK, 0, "exclude-per-directory", &opts,
> N_("gitignore"),
> N_("allow explicitly ignored files to be overwritten"),
> PARSE_OPT_NONEG, exclude_per_directory_cb },
> - OPT_SET_INT('i', NULL, &opts.index_only,
> - N_("don't check the working tree after merging"), 1),
> + OPT_BOOL('i', NULL, &opts.index_only,
> + N_("don't check the working tree after merging")),
> OPT__DRY_RUN(&opts.dry_run, N_("don't update the index or the work tree")),
> - OPT_SET_INT(0, "no-sparse-checkout", &opts.skip_sparse_checkout,
> - N_("skip applying sparse checkout filter"), 1),
> - OPT_SET_INT(0, "debug-unpack", &opts.debug_unpack,
> - N_("debug unpack-trees"), 1),
> + OPT_BOOL(0, "no-sparse-checkout", &opts.skip_sparse_checkout,
> + N_("skip applying sparse checkout filter")),
> + OPT_BOOL(0, "debug-unpack", &opts.debug_unpack,
> + N_("debug unpack-trees")),
> OPT_END()
> };
next prev parent reply other threads:[~2017-01-10 20:41 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-10 1:45 [RFC/PATCH 0/4] working tree operations: support superprefix Stefan Beller
2017-01-10 1:45 ` [PATCH 1/4] read-tree: use OPT_BOOL instead of OPT_SET_INT Stefan Beller
2017-01-10 20:41 ` Junio C Hamano [this message]
2017-01-10 1:45 ` [PATCH 2/4] t1000: modernize style Stefan Beller
2017-01-10 20:37 ` Junio C Hamano
2017-01-10 20:43 ` Stefan Beller
2017-01-10 22:01 ` Junio C Hamano
2017-01-10 1:45 ` [PATCH 3/4] t1001: " Stefan Beller
2017-01-10 1:45 ` [PATCH 4/4] unpack-trees: support super-prefix option Stefan Beller
2017-01-11 21:32 ` Junio C Hamano
2017-01-11 22:12 ` Stefan Beller
2017-01-11 23:28 ` Junio C Hamano
2017-01-11 23:57 ` Stefan Beller
2017-01-12 0:12 ` [PATCHv2 " Stefan Beller
2017-01-12 21:40 ` Junio C Hamano
2017-01-12 22:19 ` Stefan Beller
[not found] ` <152c0fbf-084c-847f-2a30-a45ea3dd26f2@gmail.com>
2017-01-13 17:56 ` [RFC/PATCH 0/4] working tree operations: support superprefix Brian J. Davis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmveyr6kk.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=bmwill@google$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=novalis@novalis$(echo .)org \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox