git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 6/6] apply, ls-files: simplify "-z" parsing
Date: Sun, 31 Jan 2016 06:35:46 -0500	[thread overview]
Message-ID: <20160131113546.GF5116@sigill.intra.peff.net> (raw)
In-Reply-To: <20160131112215.GA4589@sigill.intra.peff.net>

As a short option, we cannot handle negation. Thus a
callback handling "unset" is overkill, and we can just use
OPT_SET_INT instead to handle setting the option.

Signed-off-by: Jeff King <peff@peff.net>
---
I left this one for last, because it's the most questionable. Unlike the
previous "-z" case, we're setting the actual character, so the logic is
inverted: turning on the option sets it to 0, and turning it off restore
'\n'.

This means OPT_SET_INT would do the wrong thing for the "unset" case, as
it would put a "0" into the option. You can't trigger that now, but if
somebody were to add a long option (e.g., "--nul"), then "--no-nul"
would do the wrong thing.

I'm on the fence on whether the simplification is worth it, or if we
should leave the callbacks as future-proofing.

 builtin/apply.c    | 15 ++-------------
 builtin/ls-files.c | 13 ++-----------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index deb1364..565f3fd 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4464,16 +4464,6 @@ static int option_parse_p(const struct option *opt,
 	return 0;
 }
 
-static int option_parse_z(const struct option *opt,
-			  const char *arg, int unset)
-{
-	if (unset)
-		line_termination = '\n';
-	else
-		line_termination = 0;
-	return 0;
-}
-
 static int option_parse_space_change(const struct option *opt,
 			  const char *arg, int unset)
 {
@@ -4546,9 +4536,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 			 N_( "attempt three-way merge if a patch does not apply")),
 		OPT_FILENAME(0, "build-fake-ancestor", &fake_ancestor,
 			N_("build a temporary index based on embedded index information")),
-		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
-			N_("paths are separated with NUL character"),
-			PARSE_OPT_NOARG, option_parse_z },
+		OPT_SET_INT('z', NULL, &line_termination,
+			N_("paths are separated with NUL character"), '\0'),
 		OPT_INTEGER('C', NULL, &p_context,
 				N_("ensure at least <n> lines of context match")),
 		{ OPTION_CALLBACK, 0, "whitespace", &whitespace_option, N_("action"),
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6a7cb0..59bad9b 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -359,14 +359,6 @@ static const char * const ls_files_usage[] = {
 	NULL
 };
 
-static int option_parse_z(const struct option *opt,
-			  const char *arg, int unset)
-{
-	line_terminator = unset ? '\n' : '\0';
-
-	return 0;
-}
-
 static int option_parse_exclude(const struct option *opt,
 				const char *arg, int unset)
 {
@@ -408,9 +400,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	struct exclude_list *el;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	struct option builtin_ls_files_options[] = {
-		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
-			N_("paths are separated with NUL character"),
-			PARSE_OPT_NOARG, option_parse_z },
+		OPT_SET_INT('z', NULL, &line_terminator,
+			N_("paths are separated with NUL character"), '\0'),
 		OPT_BOOL('t', NULL, &show_tag,
 			N_("identify the file status with tags")),
 		OPT_BOOL('v', NULL, &show_valid_bit,
-- 
2.7.0.489.g6faad84

  parent reply	other threads:[~2016-01-31 11:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-31 11:22 [PATCH 0/6] post-strbuf_getline cleanups Jeff King
2016-01-31 11:25 ` [PATCH 1/6] give "nbuf" strbuf a more meaningful name Jeff King
2016-01-31 11:54   ` Johannes Schindelin
2016-01-31 11:59     ` Jeff King
2016-01-31 12:01       ` Johannes Schindelin
2016-01-31 11:25 ` [PATCH 2/6] checkout-index: simplify "-z" option parsing Jeff King
2016-01-31 11:26 ` [PATCH 3/6] checkout-index: handle "--no-prefix" option Jeff King
2016-01-31 11:29 ` [PATCH 4/6] checkout-index: handle "--no-index" option Jeff King
2016-02-01  2:25   ` Junio C Hamano
2016-02-01  3:22     ` Jeff King
2016-01-31 11:30 ` [PATCH 5/6] checkout-index: disallow "--no-stage" option Jeff King
2016-02-01  2:18   ` Junio C Hamano
2016-02-01  3:18     ` Jeff King
2016-01-31 11:35 ` Jeff King [this message]
2016-01-31 11:59   ` [PATCH 6/6] apply, ls-files: simplify "-z" parsing Johannes Schindelin
2016-02-01 21:47   ` Junio C Hamano
2016-02-01 22:52     ` Junio C Hamano
2016-02-02  5:29       ` Jeff King
2016-02-01  2:14 ` [PATCH 0/6] post-strbuf_getline cleanups Junio C Hamano

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=20160131113546.GF5116@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).