{"thread":{"id":"41191","subject":"[PATCH v11] ls-files: add eol diagnostics","startedAt":"2016-01-14T16:17:21Z","lastAt":"2016-01-15T04:51:35Z","messageCount":3,"participants":["tboegi@web.de","Junio C Hamano","Torsten Bögershausen"],"isPatch":true,"patchVersion":11,"patchTotal":null},"messages":[{"id":"276045","messageId":"1452788241-9879-1-git-send-email-tboegi@web.de","threadId":"41191","inReplyTo":null,"subject":"[PATCH v11] ls-files: add eol diagnostics","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2016-01-14T16:17:21Z","receivedAt":"2016-01-14T16:17:21Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nWhen working in a cross-platform environment, a user may want to\ncheck if text files are stored normalized in the repository and\nif .gitattributes are set appropriately.\n\nMake it possible to let Git show the line endings in the index and\nin the working tree and the effective text/eol attributes.\n\nThe end of line (\"eolinfo\") are shown like this:\n\n    \"binary\"       binary file\n    \"none\"         text file without any EOL\n    \"lf\"           text file with LF\n    \"crlf\"         text file with CRLF\n    \"mixed\"        text file with mixed line endings.\n\nThe effective text/eol attribute is one of these:\n\n    \"\", \"-text\", \"text\", \"text=auto\", \"text eol=lf\", \"text eol=crlf\"\n\ngit ls-files --eol gives an output like this:\n\n    i/none   w/none   attr/text=auto      t/t5100/empty\n    i/binary w/binary attr/-text          t/test-binary-2.png\n    i/lf     w/lf     attr/text eol=lf    t/t5100/rfc2047-info-0007\n    i/lf     w/crlf   attr/text eol=crlf  doit.bat\n    i/mixed  w/mixed  attr/               locale/XX.po\n\nto show what eol convention is used in the data in the index ('i'),\nand in the working tree ('w'), and what attribute is in effect,\nfor each path that is shown.\n\nAdd test cases in t0027.\n\nHelped-By: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\nChanges since v10:\nconvert.c:\n - CONVERT_STAT_BITS_TXT_LF   (1) -> 0x\n - introduce convert_is_binary() to share code\n   (More patches likely to come later :-)\n\nDocumentation/git-ls-files.txt:\n - cleanup around <eolinfo> <eolattr>\n - document ls-files --eol -z, TAB vs SPACES\nbuiltin/ls-files.c\n  - optimize write_eolinfo\n  - document the hard coded \"i/%-6s w/%-6s attr/%-13s\" better\nGeneral:\n  \"eol=crlf\" implies text, so print it as \"text eol=crlf\"\n  The plan is to allow \"text=auto eol=crlf\" later.\n\nt0027:\n - Better indentation of the nre case-esac\n   (I may have asked that before: is there anybody who has an init.el\n   for git.git ?\n - removed src=crlf_false_attr as commented out in the review,\n - refactored the sort <e >expect\n\n\n Documentation/git-ls-files.txt |  31 ++++++++++\n builtin/ls-files.c             |  26 ++++++++\n convert.c                      | 119 +++++++++++++++++++++++++++---------\n convert.h                      |   3 +\n t/t0027-auto-crlf.sh           | 134 ++++++++++++++++++++++++++++++++++-------\n 5 files changed, 264 insertions(+), 49 deletions(-)\n\ndiff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt\nindex e26f01f..f9206f6 100644\n--- a/Documentation/git-ls-files.txt\n+++ b/Documentation/git-ls-files.txt\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git ls-files' [-z] [-t] [-v]\n \t\t(--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*\n \t\t(-[c|d|o|i|s|u|k|m])*\n+\t\t[--eol]\n \t\t[-x <pattern>|--exclude=<pattern>]\n \t\t[-X <file>|--exclude-from=<file>]\n \t\t[--exclude-per-directory=<file>]\n@@ -147,6 +148,24 @@ a space) at the start of each line:\n \tpossible for manual inspection; the exact format may change at\n \tany time.\n \n+--eol::\n+\tShow <eolinfo> and <eolattr> of files.\n+\t<eolinfo> is the file content identification used by Git when\n+\tthe \"text\" attribute is \"auto\" (or not set and core.autocrlf is not false).\n+\t<eolinfo> is either \"binary\", \"none\", \"lf\", \"crlf\" or \"mixed\" or \"\".\n++\n+\"\" means the file is not a regular file, it is not in the index or\n+not accessable in the working tree.\n++\n+<eolattr> is the attribute that is used when checking out or committing,\n+it is either \"\", \"-text\", \"text\", \"text=auto\", \"text eol=lf\", \"text eol=crlf\".\n+Note: Currently Git does not support \"text=auto eol=lf\" or \"text=auto eol=crlf\",\n+that may change in the future.\n++\n+Both the <eolinfo> in the index (\"i/<eolinfo>\")\n+and in the working tree (\"w/<eolinfo>\") are shown for regular files,\n+followed by the  (\"attr/<eolattr>\").\n+\n \\--::\n \tDo not interpret any more arguments as options.\n \n@@ -161,6 +180,18 @@ which case it outputs:\n \n         [<tag> ]<mode> <object> <stage> <file>\n \n+'git ls-files --eol' will show\n+\ti/<eolinfo><SPACES>w/<eolinfo><SPACES>attr/<eolattr><SPACES><file>\n+\n+'git ls-files --eol -z' will use a TAB instead of spaces:\n+\ti/<eolinfo><TAB>w/<eolinfo><TAB>attr/<eolattr><TAB><file>\n+\n+'git ls-files --eol -o' will show\n+\ti/          w/<eolinfo> attr/<eolattr> <file>\n+\n+'git ls-files --eol -s' will show\n+[<tag> ]<mode> <object> <stage> i/<eolinfo> w/<eolinfo> attr/<eolattr> <file>\n+\n 'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine\n detailed information on unmerged paths.\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex b6a7cb0..cd8df03 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -27,6 +27,7 @@ static int show_killed;\n static int show_valid_bit;\n static int line_terminator = '\\n';\n static int debug_mode;\n+static int show_eol;\n \n static const char *prefix;\n static int max_prefix_len;\n@@ -47,6 +48,28 @@ static const char *tag_modified = \"\";\n static const char *tag_skip_worktree = \"\";\n static const char *tag_resolve_undo = \"\";\n \n+static void write_eolinfo(const struct cache_entry *ce, const char *path)\n+{\n+\tif (!show_eol)\n+\t\treturn;\n+\telse {\n+\t\tstruct stat st;\n+\t\tconst char *i_txt = \"\";\n+\t\tconst char *w_txt = \"\";\n+\t\tconst char *a_txt = get_convert_attr_ascii(path);\n+\t\tif (ce && S_ISREG(ce->ce_mode))\n+\t\t\ti_txt = get_cached_convert_stats_ascii(ce->name);\n+\t\tif (!lstat(path, &st) && S_ISREG(st.st_mode))\n+\t\t\tw_txt = get_wt_convert_stats_ascii(path);\n+\t\t/* Align the print for the longest possible values (see convert.c)\n+\t\t\t i/binary w/binary attr/text eol=crlf */\n+\t\tif (line_terminator == '\\n')\n+\t\t\tprintf(\"i/%-6s w/%-6s attr/%-13s \", i_txt, w_txt, a_txt);\n+\t\telse\n+\t\t\tprintf(\"i/%s\\tw/%s\\tattr/%s\\t\", i_txt, w_txt, a_txt);\n+\t}\n+}\n+\n static void write_name(const char *name)\n {\n \t/*\n@@ -68,6 +91,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)\n \t\treturn;\n \n \tfputs(tag, stdout);\n+\twrite_eolinfo(NULL, ent->name);\n \twrite_name(ent->name);\n }\n \n@@ -170,6 +194,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)\n \t\t       find_unique_abbrev(ce->sha1,abbrev),\n \t\t       ce_stage(ce));\n \t}\n+\twrite_eolinfo(ce, ce->name);\n \twrite_name(ce->name);\n \tif (debug_mode) {\n \t\tconst struct stat_data *sd = &ce->ce_stat_data;\n@@ -433,6 +458,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\tOPT_BIT(0, \"directory\", &dir.flags,\n \t\t\tN_(\"show 'other' directories' names only\"),\n \t\t\tDIR_SHOW_OTHER_DIRECTORIES),\n+\t\tOPT_BOOL(0, \"eol\", &show_eol, N_(\"show line endings of files\")),\n \t\tOPT_NEGBIT(0, \"empty-directory\", &dir.flags,\n \t\t\tN_(\"don't show empty directories\"),\n \t\t\tDIR_HIDE_EMPTY_DIRECTORIES),\ndiff --git a/convert.c b/convert.c\nindex 814e814..ae766bd 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -13,6 +13,11 @@\n  * translation when the \"text\" attribute or \"auto_crlf\" option is set.\n  */\n \n+/* Stat bits: When BIN is set, the txt bits are unset */\n+#define CONVERT_STAT_BITS_TXT_LF    0x1\n+#define CONVERT_STAT_BITS_TXT_CRLF  0x2\n+#define CONVERT_STAT_BITS_BIN       0x4\n+\n enum crlf_action {\n \tCRLF_GUESS = -1,\n \tCRLF_BINARY = 0,\n@@ -75,26 +80,75 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *\n \n /*\n  * The same heuristics as diff.c::mmfile_is_binary()\n+ * We treat files with bare CR as binary\n  */\n-static int is_binary(unsigned long size, struct text_stat *stats)\n+static inline int convert_is_binary(unsigned long size, const struct text_stat *stats)\n {\n-\n+\tif (stats->cr != stats->crlf)\n+\t\treturn 1;\n \tif (stats->nul)\n \t\treturn 1;\n \tif ((stats->printable >> 7) < stats->nonprintable)\n \t\treturn 1;\n-\t/*\n-\t * Other heuristics? Average line length might be relevant,\n-\t * as might LF vs CR vs CRLF counts..\n-\t *\n-\t * NOTE! It might be normal to have a low ratio of CRLF to LF\n-\t * (somebody starts with a LF-only file and edits it with an editor\n-\t * that adds CRLF only to lines that are added..). But do  we\n-\t * want to support CR-only? Probably not.\n-\t */\n \treturn 0;\n }\n \n+static unsigned int gather_convert_stats(const char *data, unsigned long size)\n+{\n+\tstruct text_stat stats;\n+\tif (!data || !size)\n+\t\treturn 0;\n+\tgather_stats(data, size, &stats);\n+\tif (convert_is_binary(size, &stats))\n+\t\treturn CONVERT_STAT_BITS_BIN;\n+\telse if (stats.crlf && stats.crlf == stats.lf)\n+\t\treturn CONVERT_STAT_BITS_TXT_CRLF;\n+\telse if (stats.crlf && stats.lf)\n+\t\treturn CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF;\n+\telse if (stats.lf)\n+\t\treturn CONVERT_STAT_BITS_TXT_LF;\n+\telse\n+\t\treturn 0;\n+}\n+\n+static const char *gather_convert_stats_ascii(const char *data, unsigned long size)\n+{\n+\tunsigned int convert_stats = gather_convert_stats(data, size);\n+\n+\tif (convert_stats & CONVERT_STAT_BITS_BIN)\n+\t\treturn \"binary\";\n+\tswitch (convert_stats) {\n+\tcase CONVERT_STAT_BITS_TXT_LF:\n+\t\treturn \"lf\";\n+\tcase CONVERT_STAT_BITS_TXT_CRLF:\n+\t\treturn \"crlf\";\n+\tcase CONVERT_STAT_BITS_TXT_LF | CONVERT_STAT_BITS_TXT_CRLF:\n+\t\treturn \"mixed\";\n+\tdefault:\n+\t\treturn \"none\";\n+\t}\n+}\n+\n+const char *get_cached_convert_stats_ascii(const char *path)\n+{\n+\tconst char *ret;\n+\tunsigned long sz;\n+\tvoid *data = read_blob_data_from_cache(path, &sz);\n+\tret = gather_convert_stats_ascii(data, sz);\n+\tfree(data);\n+\treturn ret;\n+}\n+\n+const char *get_wt_convert_stats_ascii(const char *path)\n+{\n+\tconst char *ret = \"\";\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tif (strbuf_read_file(&sb, path, 0) >= 0)\n+\t\tret = gather_convert_stats_ascii(sb.buf, sb.len);\n+\tstrbuf_release(&sb);\n+\treturn ret;\n+}\n+\n static enum eol output_eol(enum crlf_action crlf_action)\n {\n \tswitch (crlf_action) {\n@@ -187,18 +241,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,\n \tgather_stats(src, len, &stats);\n \n \tif (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {\n-\t\t/*\n-\t\t * We're currently not going to even try to convert stuff\n-\t\t * that has bare CR characters. Does anybody do that crazy\n-\t\t * stuff?\n-\t\t */\n-\t\tif (stats.cr != stats.crlf)\n-\t\t\treturn 0;\n-\n-\t\t/*\n-\t\t * And add some heuristics for binary vs text, of course...\n-\t\t */\n-\t\tif (is_binary(len, &stats))\n+\t\tif (convert_is_binary(len, &stats))\n \t\t\treturn 0;\n \n \t\tif (crlf_action == CRLF_GUESS) {\n@@ -277,11 +320,7 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,\n \t\t\t\treturn 0;\n \t\t}\n \n-\t\t/* If we have any bare CR characters, we're not going to touch it */\n-\t\tif (stats.cr != stats.crlf)\n-\t\t\treturn 0;\n-\n-\t\tif (is_binary(len, &stats))\n+\t\tif (convert_is_binary(len, &stats))\n \t\t\treturn 0;\n \t}\n \n@@ -777,6 +816,30 @@ int would_convert_to_git_filter_fd(const char *path)\n \treturn apply_filter(path, NULL, 0, -1, NULL, ca.drv->clean);\n }\n \n+const char *get_convert_attr_ascii(const char *path)\n+{\n+\tstruct conv_attrs ca;\n+\tenum crlf_action crlf_action;\n+\n+\tconvert_attrs(&ca, path);\n+\tcrlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);\n+\tswitch (crlf_action) {\n+\tcase CRLF_GUESS:\n+\t\treturn \"\";\n+\tcase CRLF_BINARY:\n+\t\treturn \"-text\";\n+\tcase CRLF_TEXT:\n+\t\treturn \"text\";\n+\tcase CRLF_INPUT:\n+\t\treturn \"text eol=lf\";\n+\tcase CRLF_CRLF:\n+\t\treturn \"text eol=crlf\";\n+\tcase CRLF_AUTO:\n+\t\treturn \"text=auto\";\n+\t}\n+\treturn \"\";\n+}\n+\n int convert_to_git(const char *path, const char *src, size_t len,\n                    struct strbuf *dst, enum safe_crlf checksafe)\n {\ndiff --git a/convert.h b/convert.h\nindex d9d853c..ccf436b 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -32,6 +32,9 @@ enum eol {\n };\n \n extern enum eol core_eol;\n+extern const char *get_cached_convert_stats_ascii(const char *path);\n+extern const char *get_wt_convert_stats_ascii(const char *path);\n+extern const char *get_convert_attr_ascii(const char *path);\n \n /* returns 1 if *dst was used */\n extern int convert_to_git(const char *path, const char *src, size_t len,\ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex b343651..a0cc165 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -56,21 +56,16 @@ create_gitattributes () {\n }\n \n create_NNO_files () {\n-\tlfname=$1\n-\tcrlfname=$2\n-\tlfmixcrlf=$3\n-\tlfmixcr=$4\n-\tcrlfnul=$5\n \tfor crlf in false true input\n \tdo\n \t\tfor attr in \"\" auto text -text lf crlf\n \t\tdo\n \t\t\tpfx=NNO_${crlf}_attr_${attr} &&\n-\t\t\tcp $lfname    ${pfx}_LF.txt &&\n-\t\t\tcp $crlfname  ${pfx}_CRLF.txt &&\n-\t\t\tcp $lfmixcrlf ${pfx}_CRLF_mix_LF.txt &&\n-\t\t\tcp $lfmixcr   ${pfx}_LF_mix_CR.txt &&\n-\t\t\tcp $crlfnul   ${pfx}_CRLF_nul.txt\n+\t\t\tcp CRLF_mix_LF ${pfx}_LF.txt &&\n+\t\t\tcp CRLF_mix_LF ${pfx}_CRLF.txt &&\n+\t\t\tcp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&\n+\t\t\tcp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&\n+\t\t\tcp CRLF_mix_LF ${pfx}_CRLF_nul.txt\n \t\tdone\n \tdone\n }\n@@ -96,7 +91,7 @@ commit_check_warn () {\n \tcrlfnul=$7\n \tpfx=crlf_${crlf}_attr_${attr}\n \tcreate_gitattributes \"$attr\" &&\n-\tfor f in LF CRLF repoMIX LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul\n+\tfor f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul\n \tdo\n \t\tfname=${pfx}_$f.txt &&\n \t\tcp $f $fname &&\n@@ -149,6 +144,36 @@ commit_chk_wrnNNO () {\n \t'\n }\n \n+stats_ascii () {\n+\tcase \"$1\" in\n+\tLF)\n+\t\techo lf\n+\t\t;;\n+\tCRLF)\n+\t\techo crlf\n+\t\t;;\n+\tCRLF_mix_LF)\n+\t\techo mixed\n+\t\t;;\n+\tLF_mix_CR)\n+\t\techo binary\n+\t\t;;\n+\tCRLF_nul)\n+\t\techo binary\n+\t\t;;\n+\tLF_nul)\n+\t\techo binary\n+\t\t;;\n+\tCRLF_mix_CR)\n+\t\techo binary\n+\t\t;;\n+\t*)\n+\t\techo error_invalid $1\n+\t\t;;\n+\tesac\n+\n+}\n+\n check_files_in_repo () {\n \tcrlf=$1\n \tattr=$2\n@@ -203,35 +228,84 @@ checkout_files () {\n \tcreate_gitattributes $attr &&\n \tgit config core.autocrlf $crlf &&\n \tpfx=eol_${eol}_crlf_${crlf}_attr_${attr}_ &&\n-\tsrc=crlf_false_attr__ &&\n \tfor f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul\n \tdo\n-\t\trm $src$f.txt &&\n+\t\trm crlf_false_attr__$f.txt &&\n \t\tif test -z \"$eol\"; then\n-\t\t\tgit checkout $src$f.txt\n+\t\t\tgit checkout crlf_false_attr__$f.txt\n \t\telse\n-\t\t\tgit -c core.eol=$eol checkout $src$f.txt\n+\t\t\tgit -c core.eol=$eol checkout crlf_false_attr__$f.txt\n \t\tfi\n \tdone\n \n+\ttest_expect_success \"ls-files --eol $lfname ${pfx}LF.txt\" '\n+\t\ttest_when_finished \"rm expect actual\" &&\n+\t\tsort <<-EOF >expect &&\n+\t\ti/crlf w/$(stats_ascii $crlfname) crlf_false_attr__CRLF.txt\n+\t\ti/mixed w/$(stats_ascii $lfmixcrlf) crlf_false_attr__CRLF_mix_LF.txt\n+\t\ti/lf w/$(stats_ascii $lfname) crlf_false_attr__LF.txt\n+\t\ti/binary w/$(stats_ascii $lfmixcr) crlf_false_attr__LF_mix_CR.txt\n+\t\ti/binary w/$(stats_ascii $crlfnul) crlf_false_attr__CRLF_nul.txt\n+\t\ti/binary w/$(stats_ascii $crlfnul) crlf_false_attr__LF_nul.txt\n+\t\tEOF\n+\t\tgit ls-files --eol -z crlf_false_attr__* |\n+\t\ttr \"\\000\" \"\\n\" |\n+\t\tsed -e \"s!attr/[^\t]*!!g\" -e \"s/\t/ /g\" -e \"s/  */ /g\" |\n+\t\tsort >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n \ttest_expect_success \"checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF\" \"\n-\t\tcompare_ws_file $pfx $lfname    ${src}LF.txt\n+\t\tcompare_ws_file $pfx $lfname    crlf_false_attr__LF.txt\n \t\"\n \ttest_expect_success \"checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=CRLF\" \"\n-\t\tcompare_ws_file $pfx $crlfname  ${src}CRLF.txt\n+\t\tcompare_ws_file $pfx $crlfname  crlf_false_attr__CRLF.txt\n \t\"\n \ttest_expect_success \"checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=CRLF_mix_LF\" \"\n-\t\tcompare_ws_file $pfx $lfmixcrlf ${src}CRLF_mix_LF.txt\n+\t\tcompare_ws_file $pfx $lfmixcrlf crlf_false_attr__CRLF_mix_LF.txt\n \t\"\n \ttest_expect_success \"checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF_mix_CR\" \"\n-\t\tcompare_ws_file $pfx $lfmixcr   ${src}LF_mix_CR.txt\n+\t\tcompare_ws_file $pfx $lfmixcr   crlf_false_attr__LF_mix_CR.txt\n \t\"\n \ttest_expect_success \"checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF_nul\" \"\n-\t\tcompare_ws_file $pfx $crlfnul   ${src}LF_nul.txt\n+\t\tcompare_ws_file $pfx $crlfnul   crlf_false_attr__LF_nul.txt\n \t\"\n }\n \n-#######\n+# Test control characters\n+# NUL SOH CR EOF==^Z\n+test_expect_success 'ls-files --eol -o Text/Binary' '\n+\ttest_when_finished \"rm expect actual TeBi_*\" &&\n+\tSTRT=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA &&\n+\tSTR=$STRT$STRT$STRT$STRT &&\n+\tprintf \"${STR}BBB\\001\" >TeBi_127_S &&\n+\tprintf \"${STR}BBBB\\001\">TeBi_128_S &&\n+\tprintf \"${STR}BBB\\032\" >TeBi_127_E &&\n+\tprintf \"\\032${STR}BBB\" >TeBi_E_127 &&\n+\tprintf \"${STR}BBBB\\000\">TeBi_128_N &&\n+\tprintf \"${STR}BBB\\012\">TeBi_128_L &&\n+\tprintf \"${STR}BBB\\015\">TeBi_127_C &&\n+\tprintf \"${STR}BB\\015\\012\" >TeBi_126_CL &&\n+\tprintf \"${STR}BB\\015\\012\\015\" >TeBi_126_CLC &&\n+\tsort <<-\\EOF >expect &&\n+\ti/ w/binary TeBi_127_S\n+\ti/ w/none TeBi_128_S\n+\ti/ w/none TeBi_127_E\n+\ti/ w/binary TeBi_E_127\n+\ti/ w/binary TeBi_128_N\n+\ti/ w/lf TeBi_128_L\n+\ti/ w/binary TeBi_127_C\n+\ti/ w/crlf TeBi_126_CL\n+\ti/ w/binary TeBi_126_CLC\n+\tEOF\n+\tgit ls-files --eol -z -o |\n+\ttr \"\\000\" \"\\n\" |\n+\tsed -n -e \"/TeBi_/{s!attr/[\t]*!!g\n+\ts!\t! !g\n+\tp\n+\t}\" | sort >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'setup master' '\n \techo >.gitattributes &&\n \tgit checkout -b master &&\n@@ -480,4 +554,22 @@ checkout_files    native  true  \"lf\"      LF    CRLF  CRLF_mix_LF  LF_mix_CR\n checkout_files    native  false \"crlf\"    CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul\n checkout_files    native  true  \"crlf\"    CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul\n \n+# Should be the last test case: remove some files from the worktree\n+# This test assumes that \"rm\" can remove staged files\n+test_expect_success 'ls-files --eol -d -z' '\n+\trm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt crlf_false_attr__LF.txt .gitattributes &&\n+\tcat >expect <<-\\EOF &&\n+\ti/crlf w/ crlf_false_attr__CRLF.txt\n+\ti/lf w/ .gitattributes\n+\ti/lf w/ crlf_false_attr__LF.txt\n+\ti/mixed w/ crlf_false_attr__CRLF_mix_LF.txt\n+\tEOF\n+\tgit ls-files --eol -z -d |\n+\ttr \"\\000\" \"\\n\" |\n+\tsed -e \"s!attr/[^\t]*!!g\" -e \"s/\t/ /g\" -e \"s/  */ /g\" |\n+\tsort >actual &&\n+\ttest_cmp expect actual &&\n+\trm expect actual\n+'\n+\n test_done\n-- \n2.7.0.278.g9379adb.dirty\n"},{"id":"276071","messageId":"xmqqr3hkj6q1.fsf@gitster.mtv.corp.google.com","threadId":"41191","inReplyTo":"1452788241-9879-1-git-send-email-tboegi@web.de","subject":"Re: [PATCH v11] ls-files: add eol diagnostics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-14T19:34:14Z","receivedAt":"2016-01-14T19:34:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n>  - removed src=crlf_false_attr as commented out in the review,\n\nI probably missed that exchange; I do not recall seeing that comment.\n\n> +'git ls-files --eol' will show\n> +\ti/<eolinfo><SPACES>w/<eolinfo><SPACES>attr/<eolattr><SPACES><file>\n\nI think this should be:\n\n    i/<eolinfo><SPACES>w/<eolinfo><SPACES>attr/<eolattr><SPACES><TAB><file>\n\nWhether you like it or not, people write scripts to parse textual\noutput primarily meant for humans.  \"ls-files -s\" output, for\nexample, uses tab just before the pathname even though it uses a\nspace between the mode and the object name for this exact reason.\n\n> +\t\t/* Align the print for the longest possible values (see convert.c)\n> +\t\t\t i/binary w/binary attr/text eol=crlf */\n\n\t/*\n         * We write our multi-line comments\n         * like this.\n         */\n\n> +\t\tif (line_terminator == '\\n')\n> +\t\t\tprintf(\"i/%-6s w/%-6s attr/%-13s \", i_txt, w_txt, a_txt);\n> +\t\telse\n> +\t\t\tprintf(\"i/%s\\tw/%s\\tattr/%s\\t\", i_txt, w_txt, a_txt);\n> +\t}\n> +}\n\n\n\n> diff --git a/convert.c b/convert.c\n> index 814e814..ae766bd 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -13,6 +13,11 @@\n>   * translation when the \"text\" attribute or \"auto_crlf\" option is set.\n>   */\n>  \n> +/* Stat bits: When BIN is set, the txt bits are unset */\n> +#define CONVERT_STAT_BITS_TXT_LF    0x1\n> +#define CONVERT_STAT_BITS_TXT_CRLF  0x2\n> +#define CONVERT_STAT_BITS_BIN       0x4\n> +\n>  enum crlf_action {\n>  \tCRLF_GUESS = -1,\n>  \tCRLF_BINARY = 0,\n> @@ -75,26 +80,75 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *\n>  \n>  /*\n>   * The same heuristics as diff.c::mmfile_is_binary()\n> + * We treat files with bare CR as binary\n>   */\n> -static int is_binary(unsigned long size, struct text_stat *stats)\n> +static inline int convert_is_binary(unsigned long size, const struct text_stat *stats)\n\nPlease articulate the justification for this inlining with data (or\njust keep it out-of-line as before).\n\n>  {\n> -\n> +\tif (stats->cr != stats->crlf)\n> +\t\treturn 1;\n>  \tif (stats->nul)\n>  \t\treturn 1;\n>  \tif ((stats->printable >> 7) < stats->nonprintable)\n>  \t\treturn 1;\n> -\t/*\n> -\t * Other heuristics? Average line length might be relevant,\n> -\t * as might LF vs CR vs CRLF counts..\n> -\t *\n> -\t * NOTE! It might be normal to have a low ratio of CRLF to LF\n> -\t * (somebody starts with a LF-only file and edits it with an editor\n> -\t * that adds CRLF only to lines that are added..). But do  we\n> -\t * want to support CR-only? Probably not.\n> -\t */\n>  \treturn 0;\n>  }\n>  \n> +static unsigned int gather_convert_stats(const char *data, unsigned long size)\n> +{\n> +\tstruct text_stat stats;\n> +\tif (!data || !size)\n> +\t\treturn 0;\n> +\tgather_stats(data, size, &stats);\n> +\tif (convert_is_binary(size, &stats))\n> +\t\treturn CONVERT_STAT_BITS_BIN;\n> +\telse if (stats.crlf && stats.crlf == stats.lf)\n> +\t\treturn CONVERT_STAT_BITS_TXT_CRLF;\n> +\telse if (stats.crlf && stats.lf)\n> +\t\treturn CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF;\n> +\telse if (stats.lf)\n> +\t\treturn CONVERT_STAT_BITS_TXT_LF;\n> +\telse\n> +\t\treturn 0;\n> +}\n> +\n> +static const char *gather_convert_stats_ascii(const char *data, unsigned long size)\n> +{\n> +\tunsigned int convert_stats = gather_convert_stats(data, size);\n> +\n> +\tif (convert_stats & CONVERT_STAT_BITS_BIN)\n> +\t\treturn \"binary\";\n\nIt is true and correct that we do not do EOL conversion on text\nfiles that have a lone CR, but I think it is misleading to tell the\nusers that such files are \"binary\".  We do not refrain from showing\nthe textual diff for such files, for example.\n\nTo put it another way, we do not do EOL conversion for truly\n'binary' files, but there are (mostly) text files that are not\nbinary that we do not do EOL conversion on.  And you want to tell\nthe user if EOL conversion would happen to each file.  It is not\ncorrect to label \"this file is binary\" merely because you do not do\nEOL conversion.  Perhaps define a new \"literal\" class that is a\nsuperset of \"binary\" and use that as the label?  I am not suggesting\nthat \"ls-files --eol\" should show \"i/binary\" for truly binary files\nand \"i/literal\" for a non-binary file with lone CRs.  For the\npurpose of \"--eol\", you only care about \"literal\", so you do not\neven have to have \"binary\" class at all.\n\n> +# Should be the last test case: remove some files from the worktree\n> +# This test assumes that \"rm\" can remove staged files\n\nI do not see the point of the second line.  In what situation is\n\"rm\" allowed to refuse to remove files that are added to the index?\n\n> +test_expect_success 'ls-files --eol -d -z' '\n> +\trm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt crlf_false_attr__LF.txt .gitattributes &&\n> +\tcat >expect <<-\\EOF &&\n> +\ti/crlf w/ crlf_false_attr__CRLF.txt\n> +\ti/lf w/ .gitattributes\n> +\ti/lf w/ crlf_false_attr__LF.txt\n> +\ti/mixed w/ crlf_false_attr__CRLF_mix_LF.txt\n> +\tEOF\n> +\tgit ls-files --eol -z -d |\n> +\ttr \"\\000\" \"\\n\" |\n> +\tsed -e \"s!attr/[^\t]*!!g\" -e \"s/\t/ /g\" -e \"s/  */ /g\" |\n> +\tsort >actual &&\n> +\ttest_cmp expect actual &&\n> +\trm expect actual\n\nPlease do not add useless \"clean-up\" like this last \"rm\".\n\n> +'\n> +\n>  test_done\n"},{"id":"276122","messageId":"56987AD7.2050707@web.de","threadId":"41191","inReplyTo":"xmqqr3hkj6q1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v11] ls-files: add eol diagnostics","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-15T04:51:35Z","receivedAt":"2016-01-15T04:51:35Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 01/14/2016 08:34 PM, Junio C Hamano wrote:\n(OK with the rest of the comments, thanks)\n> +\t\treturn \"binary\";\n> It is true and correct that we do not do EOL conversion on text\n> files that have a lone CR, but I think it is misleading to tell the\n> users that such files are \"binary\".  We do not refrain from showing\n> the textual diff for such files, for example.\n>\n> To put it another way, we do not do EOL conversion for truly\n> 'binary' files, but there are (mostly) text files that are not\n> binary that we do not do EOL conversion on.  And you want to tell\n> the user if EOL conversion would happen to each file.  It is not\n> correct to label \"this file is binary\" merely because you do not do\n> EOL conversion.  Perhaps define a new \"literal\" class that is a\n> superset of \"binary\" and use that as the label?  I am not suggesting\n> that \"ls-files --eol\" should show \"i/binary\" for truly binary files\n> and \"i/literal\" for a non-binary file with lone CRs.  For the\n> purpose of \"--eol\", you only care about \"literal\", so you do not\n> even have to have \"binary\" class at all.\nThis makes sense, how about \"-text\" ?\n"}]}