[PATCH 0/4] Make check-{attr,ignore} -z consistent

[PATCH 0/4] Make check-{attr,ignore} -z consistent

[Junio C Hamano]

A command that has to deal with input/output that may contain LF
needs to offer the "-z" (--nul-terminated-records) option, and if it
does not support separate --nul-terminated-{input,output} options,
the "-z" option should govern both input and output.  A caller that
uses "-z" knows that the paths it feeds to these commands as input
may have LF that cannot be expressed in LF delimited input format,
and the output from these commands do contain the same paths, so
there is no way for their output to be expressed unambiguously for
an input that requires "-z".

Unfortunately, "git check-attr -z" was broken and ignored the option
on the output side.  This is a backward-incompatible fix, so we may
need to add a "checkAttr.brokenZ" configuration to allow people to
keep the existing breakage on top of these fixes, and then flip the
default at Git 2.0 boundary (sometime early next year).

Credit goes to Eric Sunshine for finding this discrepancy
($gmane/230158).

Junio C Hamano (4):
  check-ignore: the name of the character is NUL, not NULL
  check-attr: the name of the character is NUL, not NULL
  check-ignore -z: a single -z should apply to both input and output
  check-attr -z: a single -z should apply to both input and output

 Documentation/git-check-attr.txt |  9 +++++++--
 builtin/check-attr.c             | 20 ++++++++++++++------
 builtin/check-ignore.c           | 12 ++++++------
 3 files changed, 27 insertions(+), 14 deletions(-)

-- 
1.8.3.2-911-g2c4daa5

[PATCH 1/4] check-ignore: the name of the character is NUL, not NULL

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/check-ignore.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 0240f99..be22bce 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -12,7 +12,7 @@ static const char * const check_ignore_usage[] = {
 NULL
 };
 
-static int null_term_line;
+static int nul_term_line;
 
 static const struct option check_ignore_options[] = {
 	OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -20,8 +20,8 @@ static const struct option check_ignore_options[] = {
 	OPT_GROUP(""),
 	OPT_BOOLEAN(0, "stdin", &stdin_paths,
 		    N_("read file names from stdin")),
-	OPT_BOOLEAN('z', NULL, &null_term_line,
-		    N_("input paths are terminated by a null character")),
+	OPT_BOOLEAN('z', NULL, &nul_term_line,
+		    N_("input paths are terminated by a NUL character")),
 	OPT_END()
 };
 
@@ -29,7 +29,7 @@ static void output_exclude(const char *path, struct exclude *exclude)
 {
 	char *bang  = exclude->flags & EXC_FLAG_NEGATIVE  ? "!" : "";
 	char *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? "/" : "";
-	if (!null_term_line) {
+	if (!nul_term_line) {
 		if (!verbose) {
 			write_name_quoted(path, stdout, '\n');
 		} else {
@@ -111,7 +111,7 @@ static int check_ignore_stdin_paths(const char *prefix)
 	struct strbuf buf, nbuf;
 	char **pathspec = NULL;
 	size_t nr = 0, alloc = 0;
-	int line_termination = null_term_line ? 0 : '\n';
+	int line_termination = nul_term_line ? 0 : '\n';
 	int num_ignored;
 
 	strbuf_init(&buf, 0);
@@ -150,7 +150,7 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)
 		if (argc > 0)
 			die(_("cannot specify pathnames with --stdin"));
 	} else {
-		if (null_term_line)
+		if (nul_term_line)
 			die(_("-z only makes sense with --stdin"));
 		if (argc == 0)
 			die(_("no path specified"));
-- 
1.8.3.2-911-g2c4daa5

[PATCH 2/4] check-attr: the name of the character is NUL, not NULL

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/check-attr.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 075d01d..7cc9b5d 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -13,14 +13,14 @@ N_("git check-attr --stdin [-z] [-a | --all | attr...] < <list-of-paths>"),
 NULL
 };
 
-static int null_term_line;
+static int nul_term_line;
 
 static const struct option check_attr_options[] = {
 	OPT_BOOLEAN('a', "all", &all_attrs, N_("report all attributes set on file")),
 	OPT_BOOLEAN(0,  "cached", &cached_attrs, N_("use .gitattributes only from the index")),
 	OPT_BOOLEAN(0 , "stdin", &stdin_paths, N_("read file names from stdin")),
-	OPT_BOOLEAN('z', NULL, &null_term_line,
-		N_("input paths are terminated by a null character")),
+	OPT_BOOLEAN('z', NULL, &nul_term_line,
+		    N_("input paths are terminated by a NUL character")),
 	OPT_END()
 };
 
@@ -65,7 +65,7 @@ static void check_attr_stdin_paths(const char *prefix, int cnt,
 	struct git_attr_check *check)
 {
 	struct strbuf buf, nbuf;
-	int line_termination = null_term_line ? 0 : '\n';
+	int line_termination = nul_term_line ? 0 : '\n';
 
 	strbuf_init(&buf, 0);
 	strbuf_init(&nbuf, 0);
-- 
1.8.3.2-911-g2c4daa5

[PATCH 3/4] check-ignore -z: a single -z should apply to both input and output

[Junio C Hamano]

Unless a command has separate --nul-terminated-{input,output}
options, the --nul-terminated-records (-z) option should apply
to both input and output for consistency.  The caller knows that its
input paths may need to be protected for LF, and the program shows
these problematic paths to its output.

The code already did the right thing.  Only the help text needs
fixing.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/check-ignore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index be22bce..03e509e 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -21,7 +21,7 @@ static const struct option check_ignore_options[] = {
 	OPT_BOOLEAN(0, "stdin", &stdin_paths,
 		    N_("read file names from stdin")),
 	OPT_BOOLEAN('z', NULL, &nul_term_line,
-		    N_("input paths are terminated by a NUL character")),
+		    N_("terminate input and output records by a NUL character")),
 	OPT_END()
 };
 
-- 
1.8.3.2-911-g2c4daa5

[PATCH 4/4] check-attr -z: a single -z should apply to both input and output

[Junio C Hamano]

Unless a command has separate --nul-terminated-{input,output}
options, the --nul-terminated-records (-z) option should apply
to both input and output for consistency.  The caller knows that its
input paths may need to be protected for LF, and the program shows
these problematic paths to its output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-check-attr.txt |  9 +++++++--
 builtin/check-attr.c             | 14 +++++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 5abdbaa..760aca9 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -31,8 +31,9 @@ OPTIONS
 	Read file names from stdin instead of from the command-line.
 
 -z::
-	Only meaningful with `--stdin`; paths are separated with a
-	NUL character instead of a linefeed character.
+	The output format is modified to be machine-parseable.
+	If `--stdin` is also given, input paths are separated
+	with a NUL character instead of a linefeed character.
 
 \--::
 	Interpret all preceding arguments as attributes and all following
@@ -48,6 +49,10 @@ OUTPUT
 The output is of the form:
 <path> COLON SP <attribute> COLON SP <info> LF
 
+unless `-z` is in effect, in which case NUL is used as delimiter:
+<path> NUL <attribute> NUL <info> NUL
+
+
 <path> is the path of a file being queried, <attribute> is an attribute
 being queried and <info> can be either:
 
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 7cc9b5d..cd46690 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -20,7 +20,7 @@ static const struct option check_attr_options[] = {
 	OPT_BOOLEAN(0,  "cached", &cached_attrs, N_("use .gitattributes only from the index")),
 	OPT_BOOLEAN(0 , "stdin", &stdin_paths, N_("read file names from stdin")),
 	OPT_BOOLEAN('z', NULL, &nul_term_line,
-		    N_("input paths are terminated by a NUL character")),
+		    N_("terminate input and output records by a NUL character")),
 	OPT_END()
 };
 
@@ -38,8 +38,16 @@ static void output_attr(int cnt, struct git_attr_check *check,
 		else if (ATTR_UNSET(value))
 			value = "unspecified";
 
-		quote_c_style(file, NULL, stdout, 0);
-		printf(": %s: %s\n", git_attr_name(check[j].attr), value);
+		if (nul_term_line) {
+			printf("%s%c" /* path */
+			       "%s%c" /* attrname */
+			       "%s%c" /* attrvalue */,
+			       file, 0, git_attr_name(check[j].attr), 0, value, 0);
+		} else {
+			quote_c_style(file, NULL, stdout, 0);
+			printf(": %s: %s\n", git_attr_name(check[j].attr), value);
+		}
+
 	}
 }
 
-- 
1.8.3.2-911-g2c4daa5

Re: [PATCH 0/4] Make check-{attr,ignore} -z consistent

[Eric Sunshine]

On Fri, Jul 12, 2013 at 2:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> A command that has to deal with input/output that may contain LF
> needs to offer the "-z" (--nul-terminated-records) option, and if it
> does not support separate --nul-terminated-{input,output} options,
> the "-z" option should govern both input and output.  A caller that
> uses "-z" knows that the paths it feeds to these commands as input
> may have LF that cannot be expressed in LF delimited input format,
> and the output from these commands do contain the same paths, so
> there is no way for their output to be expressed unambiguously for
> an input that requires "-z".

FWIW, applying to the entire series:

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

> Unfortunately, "git check-attr -z" was broken and ignored the option
> on the output side.  This is a backward-incompatible fix, so we may
> need to add a "checkAttr.brokenZ" configuration to allow people to
> keep the existing breakage on top of these fixes, and then flip the
> default at Git 2.0 boundary (sometime early next year).
>
> Credit goes to Eric Sunshine for finding this discrepancy
> ($gmane/230158).
>
> Junio C Hamano (4):
>   check-ignore: the name of the character is NUL, not NULL
>   check-attr: the name of the character is NUL, not NULL
>   check-ignore -z: a single -z should apply to both input and output
>   check-attr -z: a single -z should apply to both input and output
>
>  Documentation/git-check-attr.txt |  9 +++++++--
>  builtin/check-attr.c             | 20 ++++++++++++++------
>  builtin/check-ignore.c           | 12 ++++++------
>  3 files changed, 27 insertions(+), 14 deletions(-)
>
> --
> 1.8.3.2-911-g2c4daa5