[PATCH v10] ls-files: add eol diagnostics

[PATCH v10] ls-files: add eol diagnostics

[tboegi]

From: Torsten Bögershausen <tboegi@web.de>

When working in a cross-platform environment, a user may want to
check if text files are stored normalized in the repository and
if .gitattributes are set appropriately.

Make it possible to let Git show the line endings in the index and
in the working tree and the effective text/eol attributes.

The end of line ("eolinfo") are shown like this:

    "binary"       binary file
    "none"         text file without any EOL
    "lf"           text file with LF
    "crlf"         text file with CRLF
    "mixed"        text file with mixed line endings.

The effective text/eol attribute is one of these:

    "", "-text", "text", "text=auto", "eol=lf", "eol=crlf"

git ls-files --eol gives an output like this:

    i/none   w/none   attr/text=auto t/t5100/empty
    i/binary w/binary attr/-text     t/test-binary-2.png
    i/lf     w/lf     attr/eol=lf    t/t5100/rfc2047-info-0007
    i/lf     w/crlf   attr/eol=crlf  doit.bat
    i/mixed  w/mixed  attr/          locale/XX.po

to show what eol convention is used in the data in the index ('i'),
and in the working tree ('w'), and what attribute is in effect,
for each path that is shown.

Add test cases in t0027.

Helped-By: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Changes agains pu:
convert.c: 
 - Indent switch case
 - Introduced convert_is_binary() 

ls-files.c:
 - Early out in write_eolinfo()
 - Use TAB instead of multiple spaces when -z is used

t0027:
 - case-esac indentattion
 - cat EOF | sort, remove "e"
 - Replace " with '
Documentation/git-ls-files.txt:
 -  Try to make things clearer
 - -z turns SPACES inot TAB
 Documentation/git-ls-files.txt |  30 +++++++++++
 builtin/ls-files.c             |  24 +++++++++
 convert.c                      | 119 +++++++++++++++++++++++++++++++----------
 convert.h                      |   3 ++
 t/t0027-auto-crlf.sh           | 112 +++++++++++++++++++++++++++++++++-----
 5 files changed, 248 insertions(+), 40 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index e26f01f..03f4770 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git ls-files' [-z] [-t] [-v]
 		(--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
 		(-[c|d|o|i|s|u|k|m])*
+		[--eol]
 		[-x <pattern>|--exclude=<pattern>]
 		[-X <file>|--exclude-from=<file>]
 		[--exclude-per-directory=<file>]
@@ -147,6 +148,23 @@ a space) at the start of each line:
 	possible for manual inspection; the exact format may change at
 	any time.
 
+--eol::
+	Show <eolinfo> and <eolattr> of files.
+	<eolinfo> is the file content identification used by Git when
+	the "text" attribute is "auto" (or not set and core.autocrlf is not false).
+	<eolinfo> is either "binary", "none", "lf", "crlf" or "mixed" or "".
++
+"" means the file is not a regular file, it is not in the index or
+not accessable in the working tree.
++
+<eolattr> is the attribute that is used when checking out or committing,
+it is either "", "-text", "text", "text=auto", "eol=lf", "eol=crlf".
++
+Both the <eolinfo> in the index ("i/<eolinfo>")
+and in the working tree ("w/<eolinfo>") are shown for regular files,
+followed by the  ("attr/<eolattr>").
+Whenever a file is not a regular file, both <eolinfo> and <eolattr> are "".
+
 \--::
 	Do not interpret any more arguments as options.
 
@@ -161,6 +179,18 @@ which case it outputs:
 
         [<tag> ]<mode> <object> <stage> <file>
 
+'git ls-files --eol' will show
+	i/<eolinfo><SPACES>w/<eolinfo><SPACES>attr/<eolattr><SPACES><file>
+
+'git ls-files --eol -z' will use a TAB instead of spaces:
+	i/<eolinfo><TAB>w/<eolinfo><TAB>attr/<eolattr><TAB><file>
+
+'git ls-files --eol -o' will show
+	i/          w/<eolinfo> attr/<eolattr> <file>
+
+'git ls-files --eol -s' will show
+[<tag> ]<mode> <object> <stage> i/<eolinfo> w/<eolinfo> attr/<eolattr> <file>
+
 'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine
 detailed information on unmerged paths.
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6a7cb0..3a9716b 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -27,6 +27,7 @@ static int show_killed;
 static int show_valid_bit;
 static int line_terminator = '\n';
 static int debug_mode;
+static int show_eol;
 
 static const char *prefix;
 static int max_prefix_len;
@@ -47,6 +48,26 @@ static const char *tag_modified = "";
 static const char *tag_skip_worktree = "";
 static const char *tag_resolve_undo = "";
 
+static void write_eolinfo(const struct cache_entry *ce, const char *path)
+{
+	if (!show_eol)
+		return;
+	else {
+		struct stat st;
+		const char *i_txt = "";
+		const char *w_txt = "";
+		const char *a_txt = get_convert_attr_ascii(path);
+		if (ce && S_ISREG(ce->ce_mode))
+			i_txt = get_cached_convert_stats_ascii(ce->name);
+		if (!lstat(path, &st) && S_ISREG(st.st_mode))
+			w_txt = get_wt_convert_stats_ascii(path);
+		if (line_terminator == '\n')
+			printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt, a_txt);
+		else
+			printf("i/%s\tw/%s\tattr/%s\t", i_txt, w_txt, a_txt);
+	}
+}
+
 static void write_name(const char *name)
 {
 	/*
@@ -68,6 +89,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 		return;
 
 	fputs(tag, stdout);
+	write_eolinfo(NULL, ent->name);
 	write_name(ent->name);
 }
 
@@ -170,6 +192,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 		       find_unique_abbrev(ce->sha1,abbrev),
 		       ce_stage(ce));
 	}
+	write_eolinfo(ce, ce->name);
 	write_name(ce->name);
 	if (debug_mode) {
 		const struct stat_data *sd = &ce->ce_stat_data;
@@ -433,6 +456,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		OPT_BIT(0, "directory", &dir.flags,
 			N_("show 'other' directories' names only"),
 			DIR_SHOW_OTHER_DIRECTORIES),
+		OPT_BOOL(0, "eol", &show_eol, N_("show line endings of files")),
 		OPT_NEGBIT(0, "empty-directory", &dir.flags,
 			N_("don't show empty directories"),
 			DIR_HIDE_EMPTY_DIRECTORIES),
diff --git a/convert.c b/convert.c
index 814e814..de494f0 100644
--- a/convert.c
+++ b/convert.c
@@ -13,6 +13,11 @@
  * translation when the "text" attribute or "auto_crlf" option is set.
  */
 
+/* Stat bits: When BIN is set, the txt bits are unset */
+#define CONVERT_STAT_BITS_TXT_LF    0x1
+#define CONVERT_STAT_BITS_TXT_CRLF  0x2
+#define CONVERT_STAT_BITS_BIN       0x4
+
 enum crlf_action {
 	CRLF_GUESS = -1,
 	CRLF_BINARY = 0,
@@ -75,26 +80,75 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *
 
 /*
  * The same heuristics as diff.c::mmfile_is_binary()
+ * We treat files with bare CR as binary
  */
-static int is_binary(unsigned long size, struct text_stat *stats)
+static inline int convert_is_binary(unsigned long size, const struct text_stat *stats)
 {
-
+	if (stats->cr != stats->crlf)
+		return 1;
 	if (stats->nul)
 		return 1;
 	if ((stats->printable >> 7) < stats->nonprintable)
 		return 1;
-	/*
-	 * Other heuristics? Average line length might be relevant,
-	 * as might LF vs CR vs CRLF counts..
-	 *
-	 * NOTE! It might be normal to have a low ratio of CRLF to LF
-	 * (somebody starts with a LF-only file and edits it with an editor
-	 * that adds CRLF only to lines that are added..). But do  we
-	 * want to support CR-only? Probably not.
-	 */
 	return 0;
 }
 
+static unsigned int gather_convert_stats(const char *data, unsigned long size)
+{
+	struct text_stat stats;
+	if (!data || !size)
+		return 0;
+	gather_stats(data, size, &stats);
+	if (convert_is_binary(size, &stats))
+		return CONVERT_STAT_BITS_BIN;
+	else if (stats.crlf && stats.crlf == stats.lf)
+		return CONVERT_STAT_BITS_TXT_CRLF;
+	else if (stats.crlf && stats.lf)
+		return CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF;
+	else if (stats.lf)
+		return CONVERT_STAT_BITS_TXT_LF;
+	else
+		return 0;
+}
+
+static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+{
+	unsigned int convert_stats = gather_convert_stats(data, size);
+
+	if (convert_stats & CONVERT_STAT_BITS_BIN)
+		return "binary";
+	switch (convert_stats) {
+	case CONVERT_STAT_BITS_TXT_LF:
+		return "lf";
+	case CONVERT_STAT_BITS_TXT_CRLF:
+		return "crlf";
+	case CONVERT_STAT_BITS_TXT_LF | CONVERT_STAT_BITS_TXT_CRLF:
+		return "mixed";
+	default:
+		return "none";
+	}
+}
+
+const char *get_cached_convert_stats_ascii(const char *path)
+{
+	const char *ret;
+	unsigned long sz;
+	void *data = read_blob_data_from_cache(path, &sz);
+	ret = gather_convert_stats_ascii(data, sz);
+	free(data);
+	return ret;
+}
+
+const char *get_wt_convert_stats_ascii(const char *path)
+{
+	const char *ret = "";
+	struct strbuf sb = STRBUF_INIT;
+	if (strbuf_read_file(&sb, path, 0) >= 0)
+		ret = gather_convert_stats_ascii(sb.buf, sb.len);
+	strbuf_release(&sb);
+	return ret;
+}
+
 static enum eol output_eol(enum crlf_action crlf_action)
 {
 	switch (crlf_action) {
@@ -187,18 +241,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 	gather_stats(src, len, &stats);
 
 	if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {
-		/*
-		 * We're currently not going to even try to convert stuff
-		 * that has bare CR characters. Does anybody do that crazy
-		 * stuff?
-		 */
-		if (stats.cr != stats.crlf)
-			return 0;
-
-		/*
-		 * And add some heuristics for binary vs text, of course...
-		 */
-		if (is_binary(len, &stats))
+		if (convert_is_binary(len, &stats))
 			return 0;
 
 		if (crlf_action == CRLF_GUESS) {
@@ -277,11 +320,7 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
 				return 0;
 		}
 
-		/* If we have any bare CR characters, we're not going to touch it */
-		if (stats.cr != stats.crlf)
-			return 0;
-
-		if (is_binary(len, &stats))
+		if (convert_is_binary(len, &stats))
 			return 0;
 	}
 
@@ -777,6 +816,30 @@ int would_convert_to_git_filter_fd(const char *path)
 	return apply_filter(path, NULL, 0, -1, NULL, ca.drv->clean);
 }
 
+const char *get_convert_attr_ascii(const char *path)
+{
+	struct conv_attrs ca;
+	enum crlf_action crlf_action;
+
+	convert_attrs(&ca, path);
+	crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
+	switch (crlf_action) {
+	case CRLF_GUESS:
+		return "";
+	case CRLF_BINARY:
+		return "-text";
+	case CRLF_TEXT:
+		return "text";
+	case CRLF_INPUT:
+		return "eol=lf";
+	case CRLF_CRLF:
+		return "eol=crlf";
+	case CRLF_AUTO:
+		return "text=auto";
+	}
+	return "";
+}
+
 int convert_to_git(const char *path, const char *src, size_t len,
                    struct strbuf *dst, enum safe_crlf checksafe)
 {
diff --git a/convert.h b/convert.h
index d9d853c..ccf436b 100644
--- a/convert.h
+++ b/convert.h
@@ -32,6 +32,9 @@ enum eol {
 };
 
 extern enum eol core_eol;
+extern const char *get_cached_convert_stats_ascii(const char *path);
+extern const char *get_wt_convert_stats_ascii(const char *path);
+extern const char *get_convert_attr_ascii(const char *path);
 
 /* returns 1 if *dst was used */
 extern int convert_to_git(const char *path, const char *src, size_t len,
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index b343651..2614612 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -56,21 +56,16 @@ create_gitattributes () {
 }
 
 create_NNO_files () {
-	lfname=$1
-	crlfname=$2
-	lfmixcrlf=$3
-	lfmixcr=$4
-	crlfnul=$5
 	for crlf in false true input
 	do
 		for attr in "" auto text -text lf crlf
 		do
 			pfx=NNO_${crlf}_attr_${attr} &&
-			cp $lfname    ${pfx}_LF.txt &&
-			cp $crlfname  ${pfx}_CRLF.txt &&
-			cp $lfmixcrlf ${pfx}_CRLF_mix_LF.txt &&
-			cp $lfmixcr   ${pfx}_LF_mix_CR.txt &&
-			cp $crlfnul   ${pfx}_CRLF_nul.txt
+			cp CRLF_mix_LF ${pfx}_LF.txt &&
+			cp CRLF_mix_LF ${pfx}_CRLF.txt &&
+			cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
+			cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
+			cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
 		done
 	done
 }
@@ -96,7 +91,7 @@ commit_check_warn () {
 	crlfnul=$7
 	pfx=crlf_${crlf}_attr_${attr}
 	create_gitattributes "$attr" &&
-	for f in LF CRLF repoMIX LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
+	for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
 	do
 		fname=${pfx}_$f.txt &&
 		cp $f $fname &&
@@ -149,6 +144,36 @@ commit_chk_wrnNNO () {
 	'
 }
 
+stats_ascii () {
+	case "$1" in
+	LF)
+	echo lf
+	;;
+	CRLF)
+	echo crlf
+	;;
+	CRLF_mix_LF)
+	echo mixed
+	;;
+	LF_mix_CR)
+	echo binary
+	;;
+	CRLF_nul)
+	echo binary
+	;;
+	LF_nul)
+	echo binary
+	;;
+	CRLF_mix_CR)
+	echo binary
+	;;
+	*)
+	echo error_invalid $1
+	;;
+	esac
+
+}
+
 check_files_in_repo () {
 	crlf=$1
 	attr=$2
@@ -214,6 +239,19 @@ checkout_files () {
 		fi
 	done
 
+	test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
+		test_when_finished 'rm expect actual' &&
+		cat <<-EOF | sort >expect &&
+		i/crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
+		i/mixed w/$(stats_ascii $lfmixcrlf) ${src}CRLF_mix_LF.txt
+		i/lf w/$(stats_ascii $lfname) ${src}LF.txt
+		i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
+		i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
+		i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt
+		EOF
+		git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/  */ /g' | sort >actual &&
+		test_cmp expect actual
+	"
 	test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF" "
 		compare_ws_file $pfx $lfname    ${src}LF.txt
 	"
@@ -231,7 +269,40 @@ checkout_files () {
 	"
 }
 
-#######
+# Test control characters
+# NUL SOH CR EOF==^Z
+test_expect_success 'ls-files --eol -o Text/Binary' '
+	test_when_finished "rm expect actual TeBi_*" &&
+	STRT=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA &&
+	STR=$STRT$STRT$STRT$STRT &&
+	printf "${STR}BBB\001" >TeBi_127_S &&
+	printf "${STR}BBBB\001">TeBi_128_S &&
+	printf "${STR}BBB\032" >TeBi_127_E &&
+	printf "\032${STR}BBB" >TeBi_E_127 &&
+	printf "${STR}BBBB\000">TeBi_128_N &&
+	printf "${STR}BBB\012">TeBi_128_L &&
+	printf "${STR}BBB\015">TeBi_127_C &&
+	printf "${STR}BB\015\012" >TeBi_126_CL &&
+	printf "${STR}BB\015\012\015" >TeBi_126_CLC &&
+	cat <<-\EOF| sort >expect &&
+	i/ w/binary TeBi_127_S
+	i/ w/none TeBi_128_S
+	i/ w/none TeBi_127_E
+	i/ w/binary TeBi_E_127
+	i/ w/binary TeBi_128_N
+	i/ w/lf TeBi_128_L
+	i/ w/binary TeBi_127_C
+	i/ w/crlf TeBi_126_CL
+	i/ w/binary TeBi_126_CLC
+	EOF
+	git ls-files --eol -o |
+	sed -n -e "/TeBi_/{s!attr/[=a-z-]*!!g
+	s!  *! !g
+	p
+	}" | sort >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'setup master' '
 	echo >.gitattributes &&
 	git checkout -b master &&
@@ -480,4 +551,21 @@ checkout_files    native  true  "lf"      LF    CRLF  CRLF_mix_LF  LF_mix_CR
 checkout_files    native  false "crlf"    CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
 checkout_files    native  true  "crlf"    CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
 
+# Should be the last test case: remove some files from the worktree
+# This test assumes that "rm" can remove staged files
+test_expect_success 'ls-files --eol -d' '
+	rm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt crlf_false_attr__LF.txt .gitattributes &&
+	cat >expect <<-\EOF &&
+	i/crlf w/ crlf_false_attr__CRLF.txt
+	i/lf w/ .gitattributes
+	i/lf w/ crlf_false_attr__LF.txt
+	i/mixed w/ crlf_false_attr__CRLF_mix_LF.txt
+	EOF
+	git ls-files --eol -d |
+	sed -e "s!attr/[=a-z-]*!!g" -e "s/  */ /g" |
+	sort >actual &&
+	test_cmp expect actual &&
+	rm expect actual
+'
+
 test_done
-- 
2.7.0.278.g182263e.dirty

Re: [PATCH v10] ls-files: add eol diagnostics

[Junio C Hamano]

tboegi@web.de writes:

> Changes agains pu:
> convert.c: 
>  - Indent switch case
>  - Introduced convert_is_binary() 
>
> ls-files.c:
>  - Early out in write_eolinfo()
>  - Use TAB instead of multiple spaces when -z is used
>
> t0027:
>  - case-esac indentattion
>  - cat EOF | sort, remove "e"
>  - Replace " with '
> Documentation/git-ls-files.txt:
>  -  Try to make things clearer
>  - -z turns SPACES inot TAB

Thanks for a summary.

>  Documentation/git-ls-files.txt |  30 +++++++++++
>  builtin/ls-files.c             |  24 +++++++++
>  convert.c                      | 119 +++++++++++++++++++++++++++++++----------
>  convert.h                      |   3 ++
>  t/t0027-auto-crlf.sh           | 112 +++++++++++++++++++++++++++++++++-----
>  5 files changed, 248 insertions(+), 40 deletions(-)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index e26f01f..03f4770 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -12,6 +12,7 @@ SYNOPSIS
>  'git ls-files' [-z] [-t] [-v]
>  		(--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
>  		(-[c|d|o|i|s|u|k|m])*
> +		[--eol]
>  		[-x <pattern>|--exclude=<pattern>]
>  		[-X <file>|--exclude-from=<file>]
>  		[--exclude-per-directory=<file>]
> @@ -147,6 +148,23 @@ a space) at the start of each line:
>  	possible for manual inspection; the exact format may change at
>  	any time.
>  
> +--eol::
> +	Show <eolinfo> and <eolattr> of files.
> +	<eolinfo> is the file content identification used by Git when
> +	the "text" attribute is "auto" (or not set and core.autocrlf is not false).
> +	<eolinfo> is either "binary", "none", "lf", "crlf" or "mixed" or "".
> ++
> +"" means the file is not a regular file, it is not in the index or
> +not accessable in the working tree.
> ++
> +<eolattr> is the attribute that is used when checking out or committing,
> +it is either "", "-text", "text", "text=auto", "eol=lf", "eol=crlf".
> ++
> +Both the <eolinfo> in the index ("i/<eolinfo>")
> +and in the working tree ("w/<eolinfo>") are shown for regular files,
> +followed by the  ("attr/<eolattr>").
> +Whenever a file is not a regular file, both <eolinfo> and <eolattr> are "".

I think the last line is a subset of what you already said at the
beginning ("" means the file is not...).

> +		if (line_terminator == '\n')
> +			printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt, a_txt);

Can we do something better than these hard-coded constants?  Why
can't the "one HT between each" approach be used for both?

> +		else
> +			printf("i/%s\tw/%s\tattr/%s\t", i_txt, w_txt, a_txt);
> +	}

> +stats_ascii () {
> +	case "$1" in
> +	LF)
> +	echo lf
> +	;;

Indent the "do this thing" part one level down, i.e.

	case A in
        X)
        	do a thing
                ;;
	...
	esac

> @@ -214,6 +239,19 @@ checkout_files () {
>  		fi
>  	done
>  
> +	test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
> +		test_when_finished 'rm expect actual' &&
> +		cat <<-EOF | sort >expect &&

Do you have to cat into a pipe?

		sort <<-EOF >expect &&

In general, "don't cat a single thing into a pipe".

> +		i/crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
> +		i/mixed w/$(stats_ascii $lfmixcrlf) ${src}CRLF_mix_LF.txt
> +		i/lf w/$(stats_ascii $lfname) ${src}LF.txt
> +		i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
> +		i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
> +		i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt
> +		EOF
> +		git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/  */ /g' | sort >actual &&

Break the pipeline, like this:

		git ls-files --eol $src* |
		sed -e 's!attr/[=a-z-]*!!g' -e 's/  */ /g' |
		sort >actual &&

> +		test_cmp expect actual
> +	"

Also, does the test body need to be quoted with double quotes,
allowing the substitutions happen _before_ the test runs?  We try to
enclose the last parameter to test_expect_success in single quotes
when possible, as subsitututing the $variables while computing the
arguments to the test_expect_success function historically was a
source of an unpleasant-to-debug bug (e.g. when they made the
resulting eval'ed string become syntactically incorrect).

Re: [PATCH v10] ls-files: add eol diagnostics

[Torsten Bögershausen]

On 01/11/2016 06:48 PM, Junio C Hamano wrote:

Thanks for comments
>> +		if (line_terminator == '\n')
>> +			printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt, a_txt);
> Can we do something better than these hard-coded constants?  Why
> can't the "one HT between each" approach be used for both?
v11 will make more clear, that currently "eol=crlf" is the same as "text 
eol=crlf".
The shortest attr is then "", the longest "text eol=crlf".
Using '\t' as a separator makes the output cluttered for a human reader.

When parsing the output, the '\t' is much more convenient (and that's 
what t0027 wil use).
Is there any chance to have a selection between human-friendly and 
script-friendly ?
Would an option -e (eol info using spaces) vs --eol (eol info with TABs) 
make sence ?

Re: [PATCH v10] ls-files: add eol diagnostics

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> On 01/11/2016 06:48 PM, Junio C Hamano wrote:
>
> Thanks for comments
>>> +		if (line_terminator == '\n')
>>> +			printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt, a_txt);
>> Can we do something better than these hard-coded constants?  Why
>> can't the "one HT between each" approach be used for both?
> v11 will make more clear, that currently "eol=crlf" is the same as
> "text eol=crlf".
> The shortest attr is then "", the longest "text eol=crlf".
> Using '\t' as a separator makes the output cluttered for a human reader.
>
> When parsing the output, the '\t' is much more convenient (and that's
> what t0027 wil use).
> Is there any chance to have a selection between human-friendly and
> script-friendly ?
> Would an option -e (eol info using spaces) vs --eol (eol info with
> TABs) make sence ?

I do not think so; what you have in v10 is infinitely better.

What I found disturbing the most was these hardcoded numbers 6,6,9
in the printf format.