All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9F] ls-files: Add eol diagnostics
@ 2016-01-07 15:17 tboegi
  2016-01-07 20:33 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: tboegi @ 2016-01-07 15:17 UTC (permalink / raw)
  To: tboegi, git

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

When working in a cross-platform environment, a user wants 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

Add test cases in t0027.

Helped-By: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Re-send using git send-email,
thanks Junio for the patience
 Documentation/git-ls-files.txt |  23 +++++++++
 builtin/ls-files.c             |  19 +++++++
 convert.c                      |  85 +++++++++++++++++++++++++++++++
 convert.h                      |   3 ++
 t/t0027-auto-crlf.sh           | 112 ++++++++++++++++++++++++++++++++++++-----
 5 files changed, 230 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index e26f01f..e231266 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,19 @@ a space) at the start of each line:
 	possible for manual inspection; the exact format may change at
 	any time.
 
+--eol::
+	Show line endings (<eolinfo>) and the text/eol attributes (<texteolattr>) of files.
+	<eolinfo> is the file content identification used by Git when
+	the "text" attribute is "auto" or "" and core.autocrlf != false.
++
+<eolinfo> is either "" (when the the info is not available"), or one of "binary",
+"none", "lf", "crlf" or "mixed".
++
+The <texteolattr> can be "", "-text", "text", "text=auto", "eol=lf", "eol=crlf".
++
+Both the content in the index ("i/") and the content in the working tree ("w/")
+are shown for regular files, followed by the <texteolattr> ("attr/").
+
 \--::
 	Do not interpret any more arguments as options.
 
@@ -161,6 +175,15 @@ which case it outputs:
 
         [<tag> ]<mode> <object> <stage> <file>
 
+'git ls-files --eol' will show
+	i/<eolinfo> w/<eolinfo> attr/<eolattr> <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..73b2ba6 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,21 @@ 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)
+{
+	struct stat st;
+	const char *i_txt = "";
+	const char *w_txt = "";
+	if (!show_eol)
+		return;
+	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);
+	printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt,
+				 get_convert_attr_ascii(path));
+}
+
 static void write_name(const char *name)
 {
 	/*
@@ -68,6 +84,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 +187,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 +451,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..3b805fa 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   (1)
+#define CONVERT_STAT_BITS_TXT_CRLF (2)
+#define CONVERT_STAT_BITS_BIN      (4)
+
 enum crlf_action {
 	CRLF_GUESS = -1,
 	CRLF_BINARY = 0,
@@ -95,6 +100,62 @@ static int is_binary(unsigned long size, struct text_stat *stats)
 	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 (is_binary(size, &stats) || stats.cr != stats.crlf)
+		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) {
@@ -777,6 +838,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..082e7cd 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,20 @@ checkout_files () {
 		fi
 	done
 
+	test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
+		test_when_finished 'rm e expect actual' &&
+		cat >e <<-EOF &&
+		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
+		sort <e >expect &&
+		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 +270,41 @@ checkout_files () {
 	"
 }
 
-#######
+# Test control characters
+# NUL SOH CR EOF==^Z
+test_expect_success 'ls-files --eol -o Text/Binary' '
+	test_when_finished "rm e 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 >e <<-\EOF &&
+	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
+	sort <e >expect &&
+	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 +553,19 @@ 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.6.2.403.g6abe3ff.dirty

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v9F] ls-files: Add eol diagnostics
  2016-01-07 15:17 [PATCH v9F] ls-files: Add eol diagnostics tboegi
@ 2016-01-07 20:33 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2016-01-07 20:33 UTC (permalink / raw)
  To: tboegi; +Cc: git

tboegi@web.de writes:

> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index e26f01f..e231266 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,19 @@ a space) at the start of each line:
>  	possible for manual inspection; the exact format may change at
>  	any time.
>  
> +--eol::
> +	Show line endings (<eolinfo>) and the text/eol attributes (<texteolattr>) of files.

This line looks overly long.

I think s/<texteolattr>/<eolattr>/ is needed to match what is shown
in the example that appears later in the documentation.

> +	<eolinfo> is the file content identification used by Git when
> +	the "text" attribute is "auto" or "" and core.autocrlf != false.
> ++
> +<eolinfo> is either "" (when the the info is not available"), or one of "binary",
> +"none", "lf", "crlf" or "mixed".
> ++
> +The <texteolattr> can be "", "-text", "text", "text=auto", "eol=lf", "eol=crlf".
> ++
> +Both the content in the index ("i/") and the content in the working tree ("w/")
> +are shown for regular files, followed by the <texteolattr> ("attr/").
> +

This makes the reader wonder what should happen for non-regular
files.  Are they omitted from the output?  Are they shown but lack
these three fields?  Only i/ and w/ are missing but the attribute
that would have applied to them if they were regular files are
shown?  Something else?

> @@ -161,6 +175,15 @@ which case it outputs:
>  
>          [<tag> ]<mode> <object> <stage> <file>
>  
> +'git ls-files --eol' will show
> +	i/<eolinfo> w/<eolinfo> attr/<eolattr> <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..73b2ba6 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,21 @@ 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)
> +{
> +	struct stat st;
> +	const char *i_txt = "";
> +	const char *w_txt = "";
> +	if (!show_eol)
> +		return;
> +	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);
> +	printf("i/%-6s w/%-6s attr/%-9s ", i_txt, w_txt,
> +				 get_convert_attr_ascii(path));
> +}

This should interact with "-z" output mode in a more sensible way to
allow easier machine parsing, no?  Aligning them with "%-<N>s" forces
the parsing scripts to strip excess spaces, and separating the fields
with hardcoded SP even more so.

> diff --git a/convert.c b/convert.c
> index 814e814..3b805fa 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   (1)
> +#define CONVERT_STAT_BITS_TXT_CRLF (2)
> +#define CONVERT_STAT_BITS_BIN      (4)
> +

What are these parentheses doing aroud the constants?  Drop them.
Write the bit patterns in either hex or octal; if these are
enumerations, they should stay in decimal.

>  enum crlf_action {
>  	CRLF_GUESS = -1,
>  	CRLF_BINARY = 0,
> @@ -95,6 +100,62 @@ static int is_binary(unsigned long size, struct text_stat *stats)
>  	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 (is_binary(size, &stats) || stats.cr != stats.crlf)
> +		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;
> +}

I'd imagine that this is attempting to match the existing logic of
what Git actually does.  Shouldn't that existing logic share the
code with this function to avoid the two implemetations drifting
apart, or would that be too much work?

> +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";

These case labels and their statements are indented one level too deep.

> +		case CONVERT_STAT_BITS_TXT_CRLF:
> +			return "crlf";
> +		case CONVERT_STAT_BITS_TXT_LF | CONVERT_STAT_BITS_TXT_CRLF:
> +			return "mixed";
> +		default:
> +			return "none";
> +	}
> +}

> @@ -777,6 +838,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 "";

Likewise.

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

Likewise.

> @@ -214,6 +239,20 @@ checkout_files () {
>  		fi
>  	done
>  
> +	test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "

Hmmmm, by opening the executable part of test_expect_success with
double quote, you are interpolating the variables in it before the
test_expect_success function even starts to run.  It may happen to
be correct in this script to do so, but it adds more mental burden
to readers (and more importantly, those who will be modifying this
script in the future).

> +		test_when_finished 'rm e expect actual' &&
> +		cat >e <<-EOF &&
> +		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
> +		sort <e >expect &&

Do you even use 'e' after this step?  Why not just produce 'expect'
directly by feeding the here-doc directly to sort?

> @@ -231,7 +270,41 @@ checkout_files () {
>  	"
>  }
> ...
> +	cat >e <<-\EOF &&
> +	...
> +	EOF
> +	sort <e >expect &&

Likewise.

> +	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

Here, there is no need to sort?

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

Write each component of the pipe on a separate line, i.e.

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

Again, the only reason you might want to use "" around the
executable part of test_expect_success here is to use '' around the
sed scripts here, but I do not think there is anything funny there
that requires single quotes, i.e.

	test_expect_success 'title' '
        	...
		git ls-files --eol -d |
                sed -e "s|attr/[=a-z-]*||g" -e "s/  */ /g" |
                sort >actual &&
		..
	'

would match our existing convention better.

> +	test_cmp expect actual &&
> +	rm expect actual

Why the final "rm"?

> +"
> +
>  test_done

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-01-07 20:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07 15:17 [PATCH v9F] ls-files: Add eol diagnostics tboegi
2016-01-07 20:33 ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.