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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:41

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.
quoted hunk
 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
quoted hunk
@@ -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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:07:42

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

Thanks for comments
quoted
+		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 ?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help