{"thread":{"id":"57663","subject":"[PATCH v2] convert: clarify line ending conversion warning","startedAt":"2022-04-05T05:38:19Z","lastAt":"2022-04-08T04:54:08Z","messageCount":10,"participants":["Alex Henrie","Ævar Arnfjörð Bjarmason","Junio C Hamano","Torsten Bögershausen","Tao Klerks"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"453098","messageId":"20220405053559.1072115-1-alexhenrie24@gmail.com","threadId":"57663","inReplyTo":null,"subject":"[PATCH v2] convert: clarify line ending conversion warning","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-04-05T05:35:59Z","receivedAt":"2022-04-05T05:38:19Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"The warning about converting line endings is extremely confusing. Its\ntwo sentences each use the word \"will\" without specifying a timeframe,\nwhich makes it sound like both sentences are referring to the same\ntimeframe. On top of that, it uses the term \"original line endings\"\nwithout saying whether \"original\" means LF or CRLF.\n\nRephrase the warning to be clear about when the line endings will be\nchanged and what they will be changed to.\n\nOn a platform whose native line endings are not CRLF (e.g. Linux), the\n\"git add\" step in the following sequence triggers the warning in\nquestion:\n\n$ git config core.autocrlf true\n$ echo 'Hello world!' >hello.txt\n$ git add hello.txt\nwarning: LF will be replaced by CRLF in hello.txt\nThe file will have its original line endings in your working directory\n\nSigned-off-by: Alex Henrie \n---\nv2:\n- Quote the confusing warning in the above commit message\n- Move the file name to the beginning of the warning message\n- Put the file name in single quotes\n- Use the arguably broader verb \"touch\" instead of \"check out\"\n- Remove the period at the end of the sentence\n- Remove the second sentence entirely\n---\n convert.c | 10 ++++------\n t/t0027-auto-crlf.sh | 8 ++++----\n 2 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 8e39731efb..733e581cb9 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -195,9 +195,8 @@ static void check_global_conv_flags_eol(const char *path,\n \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n \t\t\tdie(_(\"CRLF would be replaced by LF in %s\"), path);\n \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n-\t\t\twarning(_(\"CRLF will be replaced by LF in %s.\\n\"\n-\t\t\t\t \"The file will have its original line\"\n-\t\t\t\t \" endings in your working directory\"), path);\n+\t\t\twarning(_(\"In '%s', CRLF will be replaced by LF the\"\n+\t\t\t\t \" next time Git touches it\"), path);\n \t} else if (old_stats->lonelf && !new_stats->lonelf ) {\n \t\t/*\n \t\t * CRLFs would be added by checkout\n@@ -205,9 +204,8 @@ static void check_global_conv_flags_eol(const char *path,\n \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n \t\t\tdie(_(\"LF would be replaced by CRLF in %s\"), path);\n \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n-\t\t\twarning(_(\"LF will be replaced by CRLF in %s.\\n\"\n-\t\t\t\t \"The file will have its original line\"\n-\t\t\t\t \" endings in your working directory\"), path);\n+\t\t\twarning(_(\"In '%s', LF will be replaced by CRLF the\"\n+\t\t\t\t \" next time Git touches it\"), path);\n \t}\n }\n \ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex 0feb41a23f..7f80f46393 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -77,12 +77,12 @@ create_NNO_MIX_files () {\n \n check_warning () {\n \tcase \"$1\" in\n-\tLF_CRLF) echo \"warning: LF will be replaced by CRLF\" >\"$2\".expect ;;\n-\tCRLF_LF) echo \"warning: CRLF will be replaced by LF\" >\"$2\".expect ;;\n-\t'')\t >\"$2\".expect ;;\n+\tLF_CRLF) echo \"LF will be replaced by CRLF\" >\"$2\".expect ;;\n+\tCRLF_LF) echo \"CRLF will be replaced by LF\" >\"$2\".expect ;;\n+\t'')\t >\"$2\".expect ;;\n \t*) echo >&2 \"Illegal 1\": \"$1\" ; return false ;;\n \tesac\n-\tgrep \"will be replaced by\" \"$2\" | sed -e \"s/\\(.*\\) in [^ ]*$/\\1/\" | uniq >\"$2\".actual\n+\tsed -e \"s/^.* \\([^ ]* will be replaced by [^ ]*\\) .*$/\\1/\" \"$2\" | uniq >\"$2\".actual\n \ttest_cmp \"$2\".expect \"$2\".actual\n }\n \n-- \n2.35.1\n\n"},{"id":"453107","messageId":"220405.86sfqrlvoz.gmgdl@evledraar.gmail.com","threadId":"57663","inReplyTo":"20220405053559.1072115-1-alexhenrie24@gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-05T08:45:51Z","receivedAt":"2022-04-05T09:53:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Apr 04 2022, Alex Henrie wrote:\n\nFriendly hint: just for the future: Please use the --in-reply-to of\ngit-format-patch when submitting re-rolls, so threading with the\noriginal thread won't be broken.\n\n> The warning about converting line endings is extremely confusing. Its\n> two sentences each use the word \"will\" without specifying a timeframe,\n> which makes it sound like both sentences are referring to the same\n> timeframe. On top of that, it uses the term \"original line endings\"\n> without saying whether \"original\" means LF or CRLF.\n>\n> Rephrase the warning to be clear about when the line endings will be\n> changed and what they will be changed to.\n>\n> On a platform whose native line endings are not CRLF (e.g. Linux), the\n> \"git add\" step in the following sequence triggers the warning in\n> question:\n>\n> $ git config core.autocrlf true\n> $ echo 'Hello world!' >hello.txt\n> $ git add hello.txt\n> warning: LF will be replaced by CRLF in hello.txt\n> The file will have its original line endings in your working directory\n>\n> Signed-off-by: Alex Henrie \n> ---\n> v2:\n> - Quote the confusing warning in the above commit message\n> - Move the file name to the beginning of the warning message\n\nIn case that's in response to my v1 review, I was pointing out pretty\nmuch the opposite, i.e. try with this:\n\t\n\tdiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\n\tindex 7f80f463930..26b939566d4 100755\n\t--- a/t/t0027-auto-crlf.sh\n\t+++ b/t/t0027-auto-crlf.sh\n\t@@ -100,7 +100,12 @@ commit_check_warn () {\n\t \tdo\n\t \t\tfname=${pfx}_$f.txt &&\n\t \t\tcp $f $fname &&\n\t-\t\tgit -c core.autocrlf=$crlf add $fname 2>\"${pfx}_$f.err\"\n\t+\t\tcp $f $fname &&\n\t+\t\tcp $f $fname.1 &&\n\t+\t\tcp $f $fname.2 &&\n\t+\t\tcp $f $fname.3 &&\n\t+\n\t+\t\tgit -c core.autocrlf=$crlf add $fname*\n\t \tdone &&\n\t \tgit commit -m \"core.autocrlf $crlf\" &&\n\t \tcheck_warning \"$lfname\" ${pfx}_LF.err &&\n\nThen run with -vixd, we'll without your change emit e.g.:\n\t\n\t+ git -c core.autocrlf=true add crlf_true_attr__CRLF_mix_LF.txt crlf_true_attr__CRLF_mix_LF.txt.1 crlf_true_attr__CRLF_mix_LF.txt.2 crlf_true_attr__CRLF_mix_LF.txt.3\n\twarning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.\n\tThe file will have its original line endings in your working directory\n\twarning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.1.\n\tThe file will have its original line endings in your working directory\n\twarning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.2.\n\tThe file will have its original line endings in your working directory\n\twarning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.3.\n\tThe file will have its original line endings in your working directory\n\nBut with your change applied:\n\n\t+ git -c core.autocrlf=true add crlf_true_attr__CRLF_mix_LF.txt crlf_true_attr__CRLF_mix_LF.txt.1 crlf_true_attr__CRLF_mix_LF.txt.2 crlf_true_attr__CRLF_mix_LF.txt.3\n\twarning: In 'crlf_true_attr__CRLF_mix_LF.txt', LF will be replaced by CRLF the next time Git touches it\n\twarning: In 'crlf_true_attr__CRLF_mix_LF.txt.1', LF will be replaced by CRLF the next time Git touches it\n\twarning: In 'crlf_true_attr__CRLF_mix_LF.txt.2', LF will be replaced by CRLF the next time Git touches it\n\twarning: In 'crlf_true_attr__CRLF_mix_LF.txt.3', LF will be replaced by CRLF the next time Git touches it\n\nNow, the existing message really sucks, we should really should\nbuffering this up somehow and saying \"here's the problem with these N\nfiles\\n\".\n\nSee Tao's recent e4921d877ab (tracking branches: add advice to ambiguous\nrefspec error, 2022-04-01) for an example\n\nBut having a message in the form of:\n\n %s blah blah\n\nAs opposed to:\n\n blah blah %s\n\nWhere %s is going to be expanded as a path is IMO worse, because both\nare repetative, but at least with the latter the constant part of the\nmessage is aligned.\n\nBut anyway, while I'm still iffy about this change because it seems like\na bit of improvement and a bit of regression.\n\nEspecially as we don't offer the working directory advice at all, and\nbecause the clear explanation about what happens to the file has been\nreplaced by some \"the next time Git touches it\" suggestion.\n\nWhich, if I wasn't looking at the pre- and post-image would completely\nconfuse me.\n\nAnyway.\n\nI'd much rather have us focus less on re-arranging the deck chairs on\nthe Titanic here.\n\nThe real issue is that we're emitting this long message in a loop. Now,\nI was expecting that it would be painful to do something like Tao's\nchange since we're deep in the guts of convert.c, but if I set a\nbreakpoint on warning in gdb I'll get:\n\t\n\t(gdb) bt\n\t#0 warning (warn=0x77ed12 \"In '%s', LF will be replaced by CRLF the next time Git touches it\") at usage.c:285\n\t#1 0x0000000000572479 in check_global_conv_flags_eol (path=0x8a1e84 \"LF\", old_stats=0x7fffffff7af0, new_stats=0x7fffffff7ac8, conv_flags=18) at convert.c:207\n\t#2 0x000000000056fd49 in crlf_to_git (istate=0x855f70 , path=0x8a1e84 \"LF\", src=0x8a21b0 \"$Id: \", '0' , \" $\\nLINEONE\\nLINETWO\\nLINETHREEHREE\", len=73,\n\t buf=0x7fffffff7bf8, crlf_action=CRLF_AUTO_CRLF, conv_flags=18) at convert.c:537\n\t#3 0x000000000056f88d in convert_to_git (istate=0x855f70 , path=0x8a1e84 \"LF\", src=0x8a21b0 \"$Id: \", '0' , \" $\\nLINEONE\\nLINETWO\\nLINETHREEHREE\", len=73,\n\t dst=0x7fffffff7bf8, conv_flags=18) at convert.c:1428\n\t#4 0x0000000000619a4b in index_mem (istate=0x855f70 , oid=0x7ffff7bfda98, buf=0x8a21b0, size=73, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2179\n\t#5 0x0000000000616650 in index_core (istate=0x855f70 , oid=0x7ffff7bfda98, fd=4, size=73, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2265\n\t#6 0x000000000061621c in index_fd (istate=0x855f70 , oid=0x7ffff7bfda98, fd=4, st=0x7fffffff7e88, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2313\n\t#7 0x000000000061682e in index_path (istate=0x855f70 , oid=0x7ffff7bfda98, path=0x8a1e84 \"LF\", st=0x7fffffff7e88, flags=1) at object-file.c:2334\n\t#8 0x0000000000657b26 in add_to_index (istate=0x855f70 , path=0x8a1e84 \"LF\", st=0x7fffffff7e88, flags=0) at read-cache.c:830\n\t#9 0x0000000000658183 in add_file_to_index (istate=0x855f70 , path=0x8a1e84 \"LF\", flags=0) at read-cache.c:863\n\t#10 0x000000000040a48a in add_files (dir=0x7fffffff8020, flags=0) at builtin/add.c:469\n\t#11 0x0000000000409aa1 in cmd_add (argc=428, argv=0x7fffffff8518, prefix=0x0) at builtin/add.c:681\n\t#12 0x0000000000407cbb in run_builtin (p=0x8237f0 , argc=429, argv=0x7fffffff8510) at git.c:465\n\t#13 0x0000000000406772 in handle_builtin (argc=429, argv=0x7fffffff8510) at git.c:719\n\t#14 0x0000000000407696 in run_argv (argcp=0x7fffffff83bc, argv=0x7fffffff83b0) at git.c:786\n\t#15 0x0000000000406531 in cmd_main (argc=429, argv=0x7fffffff8510) at git.c:917\n\t#16 0x000000000051202a in main (argc=432, argv=0x7fffffff84f8) at common-main.c:56\n\t(gdb)\n\nWhich was a pleseant surprise, i.e. here we're dealing with the \"struct\nindex_state\" all the way down to the warning caller.\n\nSo that seems like a good candidate for this. Let's try:\n\t\n\tdiff --git a/builtin/add.c b/builtin/add.c\n\tindex 3ffb86a4338..189a7337fa1 100644\n\t--- a/builtin/add.c\n\t+++ b/builtin/add.c\n\t@@ -447,6 +447,11 @@ static int add_files(struct dir_struct *dir, int flags)\n\t {\n\t \tint i, exit_status = 0;\n\t \tstruct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;\n\t+\tstruct string_list crlf2lf = STRING_LIST_INIT_NODUP;\n\t+\tstruct string_list lf2crlf = STRING_LIST_INIT_NODUP;\n\t+\n\t+\tthe_index.crlf2lf = &crlf2lf;\n\t+\tthe_index.lf2crlf = &lf2crlf;\n\t \n\t \tif (dir->ignored_nr) {\n\t \t\tfprintf(stderr, _(ignore_error));\n\t@@ -480,7 +485,26 @@ static int add_files(struct dir_struct *dir, int flags)\n\t \t\texit_status = 1;\n\t \t}\n\t \n\t+\tif (crlf2lf.nr) {\n\t+\t\tstruct string_list_item *item;\n\t+\n\t+\t\twarning(\"ohes noes crlf2lf:\");\n\t+\t\tfor_each_string_list_item(item, &crlf2lf)\n\t+\t\t\twarning(\"\\t%s\", item->string);\n\t+\t}\n\t+\tif (lf2crlf.nr) {\n\t+\t\tstruct string_list_item *item;\n\t+\n\t+\t\twarning(\"ohes noes lf2crlf:\");\n\t+\t\tfor_each_string_list_item(item, &lf2crlf)\n\t+\t\t\twarning(\"\\t%s\", item->string);\n\t+\t}\n\t+\n\t \tstring_list_clear(&matched_sparse_paths, 0);\n\t+\tstring_list_clear(&crlf2lf, 0);\n\t+\tstring_list_clear(&lf2crlf, 0);\n\t+\tthe_index.crlf2lf = NULL;\n\t+\tthe_index.lf2crlf = NULL;\n\t \n\t \treturn exit_status;\n\t }\n\tdiff --git a/cache.h b/cache.h\n\tindex 6226f6a8a53..3538cdebd61 100644\n\t--- a/cache.h\n\t+++ b/cache.h\n\t@@ -341,6 +341,8 @@ struct index_state {\n\t \tstruct progress *progress;\n\t \tstruct repository *repo;\n\t \tstruct pattern_list *sparse_checkout_patterns;\n\t+\tstruct string_list *crlf2lf;\n\t+\tstruct string_list *lf2crlf;\n\t };\n\t \n\t /* Name hashing */\n\tdiff --git a/convert.c b/convert.c\n\tindex 733e581cb98..e1f8386185a 100644\n\t--- a/convert.c\n\t+++ b/convert.c\n\t@@ -185,8 +185,11 @@ static enum eol output_eol(enum convert_crlf_action crlf_action)\n\t }\n\t \n\t static void check_global_conv_flags_eol(const char *path,\n\t-\t\t\t struct text_stat *old_stats, struct text_stat *new_stats,\n\t-\t\t\t int conv_flags)\n\t+\t\t\t\t\tstruct text_stat *old_stats,\n\t+\t\t\t\t\tstruct text_stat *new_stats,\n\t+\t\t\t\t\tint conv_flags,\n\t+\t\t\t\t\tstruct string_list *crlf2lf,\n\t+\t\t\t\t\tstruct string_list *lf2crlf)\n\t {\n\t \tif (old_stats->crlf && !new_stats->crlf ) {\n\t \t\t/*\n\t@@ -194,18 +197,20 @@ static void check_global_conv_flags_eol(const char *path,\n\t \t\t */\n\t \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n\t \t\t\tdie(_(\"CRLF would be replaced by LF in %s\"), path);\n\t+\t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN && crlf2lf)\n\t+\t\t\tstring_list_append(crlf2lf, path);\n\t \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n\t-\t\t\twarning(_(\"In '%s', CRLF will be replaced by LF the\"\n\t-\t\t\t\t \" next time Git touches it\"), path);\n\t+\t\t\tBUG(\"unreachable\");\n\t \t} else if (old_stats->lonelf && !new_stats->lonelf ) {\n\t \t\t/*\n\t \t\t * CRLFs would be added by checkout\n\t \t\t */\n\t \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n\t \t\t\tdie(_(\"LF would be replaced by CRLF in %s\"), path);\n\t+\t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN && lf2crlf)\n\t+\t\t\tstring_list_append(lf2crlf, path);\n\t \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n\t-\t\t\twarning(_(\"In '%s', LF will be replaced by CRLF the\"\n\t-\t\t\t\t \" next time Git touches it\"), path);\n\t+\t\t\tBUG(\"unreachable\");\n\t \t}\n\t }\n\t \n\t@@ -534,7 +539,8 @@ static int crlf_to_git(struct index_state *istate,\n\t \t\t\tnew_stats.crlf += new_stats.lonelf;\n\t \t\t\tnew_stats.lonelf = 0;\n\t \t\t}\n\t-\t\tcheck_global_conv_flags_eol(path, &stats, &new_stats, conv_flags);\n\t+\t\tcheck_global_conv_flags_eol(path, &stats, &new_stats, conv_flags,\n\t+\t\t\t\t\t istate->crlf2lf, istate->lf2crlf);\n\t \t}\n\t \tif (!convert_crlf_into_lf)\n\t \t\treturn 0;\n\nWhich, with the test change above emits:\n\n\n\t$ git reset; ~/g/git/git -c core.autocrlf=true add *\n\tUnstaged changes after reset:\n\tM .gitattributes\n\twarning: ohes noes lf2crlf:\n\twarning: CRLF_mix_LF\n\twarning: LF\n\twarning: crlf_true_attr__LF.err.expect\n\twarning: tmp\n\nNow, obviously that's crappy WIP code as far as the message goes., and\nthe BUG() will be hit by the other caller in diff.c (which hooks in just\nbefore a call to convert_to_git(), or we'd fall back on that.\n\nAlso, i18n with _() is needed, and the message needs to be prettier, but\nwith just a tiny bit of polishing (hint hint!) this would be sooo much\nbetter.\n\nYou could even use e4921d877ab as a template to add advice() output for\nthis, so we could save ourselves the verbosity/output for users who'd\nlike to configure this away.\n\nMaybe that's all a bit too much, sorry. I just get excited about\nimproving some of the crappy parts of our UX sometimes. If you don't\nwant to run with this I suppose it could be done as a follow-up.\n"},{"id":"453127","messageId":"CAMMLpeSfxMgyJvWp9zTY6vNE4FPYJyY0HwAV-yBFSodac1R-xA@mail.gmail.com","threadId":"57663","inReplyTo":"220405.86sfqrlvoz.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-04-05T16:44:50Z","receivedAt":"2022-04-05T21:41:45Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Tue, Apr 5, 2022 at 3:35 AM Ævar Arnfjörð Bjarmason wrote:\n>\n> On Mon, Apr 04 2022, Alex Henrie wrote:\n>\n> Friendly hint: just for the future: Please use the --in-reply-to of\n> git-format-patch when submitting re-rolls, so threading with the\n> original thread won't be broken.\n>\n> > The warning about converting line endings is extremely confusing. Its\n> > two sentences each use the word \"will\" without specifying a timeframe,\n> > which makes it sound like both sentences are referring to the same\n> > timeframe. On top of that, it uses the term \"original line endings\"\n> > without saying whether \"original\" means LF or CRLF.\n> >\n> > Rephrase the warning to be clear about when the line endings will be\n> > changed and what they will be changed to.\n> >\n> > On a platform whose native line endings are not CRLF (e.g. Linux), the\n> > \"git add\" step in the following sequence triggers the warning in\n> > question:\n> >\n> > $ git config core.autocrlf true\n> > $ echo 'Hello world!' >hello.txt\n> > $ git add hello.txt\n> > warning: LF will be replaced by CRLF in hello.txt\n> > The file will have its original line endings in your working directory\n> >\n> > Signed-off-by: Alex Henrie \n> > ---\n> > v2:\n> > - Quote the confusing warning in the above commit message\n> > - Move the file name to the beginning of the warning message\n>\n> In case that's in response to my v1 review, I was pointing out pretty\n> much the opposite, i.e. try with this:\n>\n> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\n> index 7f80f463930..26b939566d4 100755\n> --- a/t/t0027-auto-crlf.sh\n> +++ b/t/t0027-auto-crlf.sh\n> @@ -100,7 +100,12 @@ commit_check_warn () {\n> do\n> fname=${pfx}_$f.txt &&\n> cp $f $fname &&\n> - git -c core.autocrlf=$crlf add $fname 2>\"${pfx}_$f.err\"\n> + cp $f $fname &&\n> + cp $f $fname.1 &&\n> + cp $f $fname.2 &&\n> + cp $f $fname.3 &&\n> +\n> + git -c core.autocrlf=$crlf add $fname*\n> done &&\n> git commit -m \"core.autocrlf $crlf\" &&\n> check_warning \"$lfname\" ${pfx}_LF.err &&\n>\n> Then run with -vixd, we'll without your change emit e.g.:\n>\n> + git -c core.autocrlf=true add crlf_true_attr__CRLF_mix_LF.txt crlf_true_attr__CRLF_mix_LF.txt.1 crlf_true_attr__CRLF_mix_LF.txt.2 crlf_true_attr__CRLF_mix_LF.txt.3\n> warning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.\n> The file will have its original line endings in your working directory\n> warning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.1.\n> The file will have its original line endings in your working directory\n> warning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.2.\n> The file will have its original line endings in your working directory\n> warning: LF will be replaced by CRLF in crlf_true_attr__CRLF_mix_LF.txt.3.\n> The file will have its original line endings in your working directory\n>\n> But with your change applied:\n>\n> + git -c core.autocrlf=true add crlf_true_attr__CRLF_mix_LF.txt crlf_true_attr__CRLF_mix_LF.txt.1 crlf_true_attr__CRLF_mix_LF.txt.2 crlf_true_attr__CRLF_mix_LF.txt.3\n> warning: In 'crlf_true_attr__CRLF_mix_LF.txt', LF will be replaced by CRLF the next time Git touches it\n> warning: In 'crlf_true_attr__CRLF_mix_LF.txt.1', LF will be replaced by CRLF the next time Git touches it\n> warning: In 'crlf_true_attr__CRLF_mix_LF.txt.2', LF will be replaced by CRLF the next time Git touches it\n> warning: In 'crlf_true_attr__CRLF_mix_LF.txt.3', LF will be replaced by CRLF the next time Git touches it\n>\n> Now, the existing message really sucks, we should really should\n> buffering this up somehow and saying \"here's the problem with these N\n> files\\n\".\n>\n> See Tao's recent e4921d877ab (tracking branches: add advice to ambiguous\n> refspec error, 2022-04-01) for an example\n>\n> But having a message in the form of:\n>\n> %s blah blah\n>\n> As opposed to:\n>\n> blah blah %s\n>\n> Where %s is going to be expanded as a path is IMO worse, because both\n> are repetative, but at least with the latter the constant part of the\n> message is aligned.\n>\n> But anyway, while I'm still iffy about this change because it seems like\n> a bit of improvement and a bit of regression.\n>\n> Especially as we don't offer the working directory advice at all, and\n> because the clear explanation about what happens to the file has been\n> replaced by some \"the next time Git touches it\" suggestion.\n>\n> Which, if I wasn't looking at the pre- and post-image would completely\n> confuse me.\n>\n> Anyway.\n>\n> I'd much rather have us focus less on re-arranging the deck chairs on\n> the Titanic here.\n>\n> The real issue is that we're emitting this long message in a loop. Now,\n> I was expecting that it would be painful to do something like Tao's\n> change since we're deep in the guts of convert.c, but if I set a\n> breakpoint on warning in gdb I'll get:\n>\n> (gdb) bt\n> #0 warning (warn=0x77ed12 \"In '%s', LF will be replaced by CRLF the next time Git touches it\") at usage.c:285\n> #1 0x0000000000572479 in check_global_conv_flags_eol (path=0x8a1e84 \"LF\", old_stats=0x7fffffff7af0, new_stats=0x7fffffff7ac8, conv_flags=18) at convert.c:207\n> #2 0x000000000056fd49 in crlf_to_git (istate=0x855f70 , path=0x8a1e84 \"LF\", src=0x8a21b0 \"$Id: \", '0' , \" $\\nLINEONE\\nLINETWO\\nLINETHREEHREE\", len=73,\n> buf=0x7fffffff7bf8, crlf_action=CRLF_AUTO_CRLF, conv_flags=18) at convert.c:537\n> #3 0x000000000056f88d in convert_to_git (istate=0x855f70 , path=0x8a1e84 \"LF\", src=0x8a21b0 \"$Id: \", '0' , \" $\\nLINEONE\\nLINETWO\\nLINETHREEHREE\", len=73,\n> dst=0x7fffffff7bf8, conv_flags=18) at convert.c:1428\n> #4 0x0000000000619a4b in index_mem (istate=0x855f70 , oid=0x7ffff7bfda98, buf=0x8a21b0, size=73, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2179\n> #5 0x0000000000616650 in index_core (istate=0x855f70 , oid=0x7ffff7bfda98, fd=4, size=73, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2265\n> #6 0x000000000061621c in index_fd (istate=0x855f70 , oid=0x7ffff7bfda98, fd=4, st=0x7fffffff7e88, type=OBJ_BLOB, path=0x8a1e84 \"LF\", flags=1) at object-file.c:2313\n> #7 0x000000000061682e in index_path (istate=0x855f70 , oid=0x7ffff7bfda98, path=0x8a1e84 \"LF\", st=0x7fffffff7e88, flags=1) at object-file.c:2334\n> #8 0x0000000000657b26 in add_to_index (istate=0x855f70 , path=0x8a1e84 \"LF\", st=0x7fffffff7e88, flags=0) at read-cache.c:830\n> #9 0x0000000000658183 in add_file_to_index (istate=0x855f70 , path=0x8a1e84 \"LF\", flags=0) at read-cache.c:863\n> #10 0x000000000040a48a in add_files (dir=0x7fffffff8020, flags=0) at builtin/add.c:469\n> #11 0x0000000000409aa1 in cmd_add (argc=428, argv=0x7fffffff8518, prefix=0x0) at builtin/add.c:681\n> #12 0x0000000000407cbb in run_builtin (p=0x8237f0 , argc=429, argv=0x7fffffff8510) at git.c:465\n> #13 0x0000000000406772 in handle_builtin (argc=429, argv=0x7fffffff8510) at git.c:719\n> #14 0x0000000000407696 in run_argv (argcp=0x7fffffff83bc, argv=0x7fffffff83b0) at git.c:786\n> #15 0x0000000000406531 in cmd_main (argc=429, argv=0x7fffffff8510) at git.c:917\n> #16 0x000000000051202a in main (argc=432, argv=0x7fffffff84f8) at common-main.c:56\n> (gdb)\n>\n> Which was a pleseant surprise, i.e. here we're dealing with the \"struct\n> index_state\" all the way down to the warning caller.\n>\n> So that seems like a good candidate for this. Let's try:\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 3ffb86a4338..189a7337fa1 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -447,6 +447,11 @@ static int add_files(struct dir_struct *dir, int flags)\n> {\n> int i, exit_status = 0;\n> struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;\n> + struct string_list crlf2lf = STRING_LIST_INIT_NODUP;\n> + struct string_list lf2crlf = STRING_LIST_INIT_NODUP;\n> +\n> + the_index.crlf2lf = &crlf2lf;\n> + the_index.lf2crlf = &lf2crlf;\n>\n> if (dir->ignored_nr) {\n> fprintf(stderr, _(ignore_error));\n> @@ -480,7 +485,26 @@ static int add_files(struct dir_struct *dir, int flags)\n> exit_status = 1;\n> }\n>\n> + if (crlf2lf.nr) {\n> + struct string_list_item *item;\n> +\n> + warning(\"ohes noes crlf2lf:\");\n> + for_each_string_list_item(item, &crlf2lf)\n> + warning(\"\\t%s\", item->string);\n> + }\n> + if (lf2crlf.nr) {\n> + struct string_list_item *item;\n> +\n> + warning(\"ohes noes lf2crlf:\");\n> + for_each_string_list_item(item, &lf2crlf)\n> + warning(\"\\t%s\", item->string);\n> + }\n> +\n> string_list_clear(&matched_sparse_paths, 0);\n> + string_list_clear(&crlf2lf, 0);\n> + string_list_clear(&lf2crlf, 0);\n> + the_index.crlf2lf = NULL;\n> + the_index.lf2crlf = NULL;\n>\n> return exit_status;\n> }\n> diff --git a/cache.h b/cache.h\n> index 6226f6a8a53..3538cdebd61 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -341,6 +341,8 @@ struct index_state {\n> struct progress *progress;\n> struct repository *repo;\n> struct pattern_list *sparse_checkout_patterns;\n> + struct string_list *crlf2lf;\n> + struct string_list *lf2crlf;\n> };\n>\n> /* Name hashing */\n> diff --git a/convert.c b/convert.c\n> index 733e581cb98..e1f8386185a 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -185,8 +185,11 @@ static enum eol output_eol(enum convert_crlf_action crlf_action)\n> }\n>\n> static void check_global_conv_flags_eol(const char *path,\n> - struct text_stat *old_stats, struct text_stat *new_stats,\n> - int conv_flags)\n> + struct text_stat *old_stats,\n> + struct text_stat *new_stats,\n> + int conv_flags,\n> + struct string_list *crlf2lf,\n> + struct string_list *lf2crlf)\n> {\n> if (old_stats->crlf && !new_stats->crlf ) {\n> /*\n> @@ -194,18 +197,20 @@ static void check_global_conv_flags_eol(const char *path,\n> */\n> if (conv_flags & CONV_EOL_RNDTRP_DIE)\n> die(_(\"CRLF would be replaced by LF in %s\"), path);\n> + else if (conv_flags & CONV_EOL_RNDTRP_WARN && crlf2lf)\n> + string_list_append(crlf2lf, path);\n> else if (conv_flags & CONV_EOL_RNDTRP_WARN)\n> - warning(_(\"In '%s', CRLF will be replaced by LF the\"\n> - \" next time Git touches it\"), path);\n> + BUG(\"unreachable\");\n> } else if (old_stats->lonelf && !new_stats->lonelf ) {\n> /*\n> * CRLFs would be added by checkout\n> */\n> if (conv_flags & CONV_EOL_RNDTRP_DIE)\n> die(_(\"LF would be replaced by CRLF in %s\"), path);\n> + else if (conv_flags & CONV_EOL_RNDTRP_WARN && lf2crlf)\n> + string_list_append(lf2crlf, path);\n> else if (conv_flags & CONV_EOL_RNDTRP_WARN)\n> - warning(_(\"In '%s', LF will be replaced by CRLF the\"\n> - \" next time Git touches it\"), path);\n> + BUG(\"unreachable\");\n> }\n> }\n>\n> @@ -534,7 +539,8 @@ static int crlf_to_git(struct index_state *istate,\n> new_stats.crlf += new_stats.lonelf;\n> new_stats.lonelf = 0;\n> }\n> - check_global_conv_flags_eol(path, &stats, &new_stats, conv_flags);\n> + check_global_conv_flags_eol(path, &stats, &new_stats, conv_flags,\n> + istate->crlf2lf, istate->lf2crlf);\n> }\n> if (!convert_crlf_into_lf)\n> return 0;\n>\n> Which, with the test change above emits:\n>\n>\n> $ git reset; ~/g/git/git -c core.autocrlf=true add *\n> Unstaged changes after reset:\n> M .gitattributes\n> warning: ohes noes lf2crlf:\n> warning: CRLF_mix_LF\n> warning: LF\n> warning: crlf_true_attr__LF.err.expect\n> warning: tmp\n>\n> Now, obviously that's crappy WIP code as far as the message goes., and\n> the BUG() will be hit by the other caller in diff.c (which hooks in just\n> before a call to convert_to_git(), or we'd fall back on that.\n>\n> Also, i18n with _() is needed, and the message needs to be prettier, but\n> with just a tiny bit of polishing (hint hint!) this would be sooo much\n> better.\n>\n> You could even use e4921d877ab as a template to add advice() output for\n> this, so we could save ourselves the verbosity/output for users who'd\n> like to configure this away.\n>\n> Maybe that's all a bit too much, sorry. I just get excited about\n> improving some of the crappy parts of our UX sometimes. If you don't\n> want to run with this I suppose it could be done as a follow-up.\n\nThanks for the extensive feedback. So, there are two problems here:\n\n1. The current warning is very confusingly worded\n\n2. The current warning is repetitive if there are a lot of files\n\nFor the moment, I'm only trying to address problem #1. Junio asked for\nthe file name to be moved to near the beginning of the message and for\nthe second sentence to be removed. Tao asked to avoid the verb \"check\nout\" and said that \"touch\" was better. What wording would you suggest?\n\n-Alex\n"},{"id":"453190","messageId":"xmqqczhu1b8s.fsf@gitster.g","threadId":"57663","inReplyTo":"CAMMLpeSfxMgyJvWp9zTY6vNE4FPYJyY0HwAV-yBFSodac1R-xA@mail.gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-06T15:29:23Z","receivedAt":"2022-04-06T17:26:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie writes:\n\n>> Now, the existing message really sucks, we should really should\n>> buffering this up somehow and saying \"here's the problem with these N\n>> files\\n\".\n> ...\n> Thanks for the extensive feedback. So, there are two problems here:\n>\n> 1. The current warning is very confusingly worded\n>\n> 2. The current warning is repetitive if there are a lot of files\n>\n> For the moment, I'm only trying to address problem #1. Junio asked for\n> the file name to be moved to near the beginning of the message and for\n> the second sentence to be removed. Tao asked to avoid the verb \"check\n> out\" and said that \"touch\" was better. What wording would you suggest?\n\nHeh, I do not ask contributors to do anything. When there is only\none right way to write a code, I may tell them to do exactly that\nway, but that is rare. I only suggest an alternative or two and\nhave contributors to think themselves. Ah, I am asking them to\nthink for themselves in that case, but that does not count ;-)\n\nAnyway.\n\nI do not have a strong opinion between checkout and touch myself.\nSwitching to \"touch\" did make it less technically incorrect than\nsaying \"checkout\" by making it a bit more vague, which can cut both\nways, but if \"touch\" can squelch complaints from hair-splitters like\n\"I did 'git checkout' (no arguments) immediately after I saw the\nmessage but nothing changed\", that may be a good change.\n\nBetween \"we have a problem with long description in 'this file'\"\nthat pushes the most crucial information to the far right in the\noutput that is necessary to tell where to look for the problem, and\n\"in 'this file', we have a problem with long description\", I would\nimagine that readers prefer to see the former, simply because it\nlets them spot the pathname easier. The repetitiveness is an issue\nthat may need to be addressed separately, and the repetitiveness\nmight make it look as if repeating the long problem description in\nthe front part of the message is better by aligning the repetitive\nmessage and by numbing readers' mind, but I do not think that\nbenefit outweighs the downside of hiding more important information\nby pushing it far to the right.\n\nJust my two cents.\n"},{"id":"453207","messageId":"20220406180434.4zlb2bwpu6cfumta@tb-raspi4","threadId":"57663","inReplyTo":"20220405053559.1072115-1-alexhenrie24@gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2022-04-06T18:04:34Z","receivedAt":"2022-04-06T19:12:53Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Apr 04, 2022 at 11:35:59PM -0600, Alex Henrie wrote:\n> The warning about converting line endings is extremely confusing. Its\n> two sentences each use the word \"will\" without specifying a timeframe,\n> which makes it sound like both sentences are referring to the same\n> timeframe. On top of that, it uses the term \"original line endings\"\n> without saying whether \"original\" means LF or CRLF.\n>\n> Rephrase the warning to be clear about when the line endings will be\n> changed and what they will be changed to.\n>\n> On a platform whose native line endings are not CRLF (e.g. Linux), the\n> \"git add\" step in the following sequence triggers the warning in\n> question:\n>\n> $ git config core.autocrlf true\n> $ echo 'Hello world!' >hello.txt\n> $ git add hello.txt\n> warning: LF will be replaced by CRLF in hello.txt\n> The file will have its original line endings in your working directory\n>\n> Signed-off-by: Alex Henrie \n> ---\n> v2:\n> - Quote the confusing warning in the above commit message\n> - Move the file name to the beginning of the warning message\n> - Put the file name in single quotes\n> - Use the arguably broader verb \"touch\" instead of \"check out\"\n> - Remove the period at the end of the sentence\n> - Remove the second sentence entirely\n> ---\n> convert.c | 10 ++++------\n> t/t0027-auto-crlf.sh | 8 ++++----\n> 2 files changed, 8 insertions(+), 10 deletions(-)\n>\n> diff --git a/convert.c b/convert.c\n> index 8e39731efb..733e581cb9 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -195,9 +195,8 @@ static void check_global_conv_flags_eol(const char *path,\n> \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n> \t\t\tdie(_(\"CRLF would be replaced by LF in %s\"), path);\n> \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n> -\t\t\twarning(_(\"CRLF will be replaced by LF in %s.\\n\"\n> -\t\t\t\t \"The file will have its original line\"\n> -\t\t\t\t \" endings in your working directory\"), path);\n> +\t\t\twarning(_(\"In '%s', CRLF will be replaced by LF the\"\n> +\t\t\t\t \" next time Git touches it\"), path);\n\n(Somewhat late to the discussions), thanks for picking this up.\nMay be we can use \"updates\" instead of \"touches\" ?\n\n\"In '%s', CRLF will be replaced by LF the next time Git updates it\"\n\nOr may be\n\n\"In '%s', CRLF will be replaced by LF the next time a `git checkout` updates it\"\n\n\n[snip]\n"},{"id":"453208","messageId":"CAPMMpohYRTSaTcRVTJ6y=Q7+FSOcSNjHjwDvUMhVv4JO7QDB_g@mail.gmail.com","threadId":"57663","inReplyTo":"20220406180434.4zlb2bwpu6cfumta@tb-raspi4","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-04-06T18:18:24Z","receivedAt":"2022-04-06T19:21:43Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Wed, Apr 6, 2022 at 8:04 PM Torsten Bögershausen wrote:\n\n> May be we can use \"updates\" instead of \"touches\" ?\n>\n> \"In '%s', CRLF will be replaced by LF the next time Git updates it\"\n\nMakes sense to me.\n\n>\n> Or may be\n>\n> \"In '%s', CRLF will be replaced by LF the next time a `git checkout` updates it\"\n>\n\nI believe we should stay away from the word \"checkout\" because it's\nneither accurate nor clear, for at least a couple of reasons:\n1. We have long, in principle, been promoting the use of \"git switch\",\nand it's not obvious in a message like this one that this is\nconsidered to be equivalent.\n2. Files can be touched/updated by other commands/processes, like \"git\npull\" (and \"rebase\", and probably others I can't think of)\n"},{"id":"453210","messageId":"CAMMLpeRw2SMaPhesXn7Q5CGq2Kmk=eJrPBnFBnJex8-OvfSz5w@mail.gmail.com","threadId":"57663","inReplyTo":"CAPMMpohYRTSaTcRVTJ6y=Q7+FSOcSNjHjwDvUMhVv4JO7QDB_g@mail.gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-04-06T18:30:02Z","receivedAt":"2022-04-06T19:40:15Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Wed, Apr 6, 2022 at 12:18 PM Tao Klerks wrote:\n>\n> On Wed, Apr 6, 2022 at 8:04 PM Torsten Bögershausen wrote:\n>\n> > May be we can use \"updates\" instead of \"touches\" ?\n> >\n> > \"In '%s', CRLF will be replaced by LF the next time Git updates it\"\n>\n> Makes sense to me.\n\nI don't like this wording as much because \"update\" could more easily\nbe taken to mean something that happens in the index or in a commit,\nnot in the working directory. But maybe it needs more clarification\nanyway, e.g. \"In the working copy of '%s'...\"\n\nÆvar, would the phrase \"In the working copy\" resolve your concern\nabout the new message \"[not offering] the working directory advice at\nall\"?\n\n> > Or may be\n> >\n> > \"In '%s', CRLF will be replaced by LF the next time a `git checkout` updates it\"\n> >\n>\n> I believe we should stay away from the word \"checkout\" because it's\n> neither accurate nor clear, for at least a couple of reasons:\n> 1. We have long, in principle, been promoting the use of \"git switch\",\n> and it's not obvious in a message like this one that this is\n> considered to be equivalent.\n> 2. Files can be touched/updated by other commands/processes, like \"git\n> pull\" (and \"rebase\", and probably others I can't think of)\n\nI agree: Talking about the `git checkout` command is too specific.\n\n-Alex\n"},{"id":"453219","messageId":"xmqqo81exal8.fsf@gitster.g","threadId":"57663","inReplyTo":"20220406180434.4zlb2bwpu6cfumta@tb-raspi4","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-06T19:42:27Z","receivedAt":"2022-04-06T21:05:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> Or may be\n>\n> \"In '%s', CRLF will be replaced by LF the next time a `git checkout` updates it\"\n\nAs this is a warning(_(\"\")) message, usual \"after 'warning:'\nheading, the message does not begin in a capital\" rule would apply,\nI think.\n\nIt may not be \"git checkout\" that updates the path. And running\n\"git checkout\" may not necessarily update the path. \"... the next\ntime Git updates it\" would probably be more widely applicable.\n"},{"id":"453220","messageId":"xmqqk0c2xafp.fsf@gitster.g","threadId":"57663","inReplyTo":"CAPMMpohYRTSaTcRVTJ6y=Q7+FSOcSNjHjwDvUMhVv4JO7QDB_g@mail.gmail.com","subject":"Re: [PATCH v2] convert: clarify line ending conversion warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-06T19:45:46Z","receivedAt":"2022-04-06T21:06:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tao Klerks writes:\n\n> On Wed, Apr 6, 2022 at 8:04 PM Torsten Bögershausen wrote:\n>\n>> May be we can use \"updates\" instead of \"touches\" ?\n>>\n>> \"In '%s', CRLF will be replaced by LF the next time Git updates it\"\n>\n> Makes sense to me.\n>\n>>\n>> Or may be\n>>\n>> \"In '%s', CRLF will be replaced by LF the next time a `git checkout` updates it\"\n>>\n>\n> I believe we should stay away from the word \"checkout\" because it's\n> neither accurate nor clear, for at least a couple of reasons:\n> 1. We have long, in principle, been promoting the use of \"git switch\",\n> and it's not obvious in a message like this one that this is\n> considered to be equivalent.\n\nIt is not an equivalent. \"switch\" is a strict subset of \"checkout\"\nand branch switching is not the only case that would correct the\nline ending on such a file. Checking the contents out of the index\nor a named commit with \"git checkout -- path\" would also fix the\nline endings.\n\n> 2. Files can be touched/updated by other commands/processes, like \"git\n> pull\" (and \"rebase\", and probably others I can't think of)\n\nYes.\n\n"},{"id":"453311","messageId":"20220408044154.9947-1-alexhenrie24@gmail.com","threadId":"57663","inReplyTo":"20220405053559.1072115-1-alexhenrie24@gmail.com","subject":"[PATCH v3] convert: clarify line ending conversion warning","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2022-04-08T04:41:54Z","receivedAt":"2022-04-08T04:54:08Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"The warning about converting line endings is extremely confusing. Its\ntwo sentences each use the word \"will\" without specifying a timeframe,\nwhich makes it sound like both sentences are referring to the same\ntimeframe. On top of that, it uses the term \"original line endings\"\nwithout saying whether \"original\" means LF or CRLF.\n\nRephrase the warning to be clear about when the line endings will be\nchanged and what they will be changed to.\n\nOn a platform whose native line endings are not CRLF (e.g. Linux), the\n\"git add\" step in the following sequence triggers the warning in\nquestion:\n\n$ git config core.autocrlf true\n$ echo 'Hello world!' >hello.txt\n$ git add hello.txt\nwarning: LF will be replaced by CRLF in hello.txt\nThe file will have its original line endings in your working directory\n\nSigned-off-by: Alex Henrie \n---\nv3:\n- Start the warning with a lowercase letter\n- Add the phrase \"the working copy of\" - hopefully this resolves the\n last concerns about clarity\n---\n convert.c | 12 ++++++------\n t/t0027-auto-crlf.sh | 8 ++++----\n 2 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 8e39731efb..4d153729da 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -195,9 +195,9 @@ static void check_global_conv_flags_eol(const char *path,\n \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n \t\t\tdie(_(\"CRLF would be replaced by LF in %s\"), path);\n \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n-\t\t\twarning(_(\"CRLF will be replaced by LF in %s.\\n\"\n-\t\t\t\t \"The file will have its original line\"\n-\t\t\t\t \" endings in your working directory\"), path);\n+\t\t\twarning(_(\"in the working copy of '%s', CRLF will be\"\n+\t\t\t\t \" replaced by LF the next time Git touches\"\n+\t\t\t\t \" it\"), path);\n \t} else if (old_stats->lonelf && !new_stats->lonelf ) {\n \t\t/*\n \t\t * CRLFs would be added by checkout\n@@ -205,9 +205,9 @@ static void check_global_conv_flags_eol(const char *path,\n \t\tif (conv_flags & CONV_EOL_RNDTRP_DIE)\n \t\t\tdie(_(\"LF would be replaced by CRLF in %s\"), path);\n \t\telse if (conv_flags & CONV_EOL_RNDTRP_WARN)\n-\t\t\twarning(_(\"LF will be replaced by CRLF in %s.\\n\"\n-\t\t\t\t \"The file will have its original line\"\n-\t\t\t\t \" endings in your working directory\"), path);\n+\t\t\twarning(_(\"in the working copy of '%s', LF will be\"\n+\t\t\t\t \" replaced by CRLF the next time Git touches\"\n+\t\t\t\t \" it\"), path);\n \t}\n }\n \ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex 0feb41a23f..7f80f46393 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -77,12 +77,12 @@ create_NNO_MIX_files () {\n \n check_warning () {\n \tcase \"$1\" in\n-\tLF_CRLF) echo \"warning: LF will be replaced by CRLF\" >\"$2\".expect ;;\n-\tCRLF_LF) echo \"warning: CRLF will be replaced by LF\" >\"$2\".expect ;;\n-\t'')\t >\"$2\".expect ;;\n+\tLF_CRLF) echo \"LF will be replaced by CRLF\" >\"$2\".expect ;;\n+\tCRLF_LF) echo \"CRLF will be replaced by LF\" >\"$2\".expect ;;\n+\t'')\t >\"$2\".expect ;;\n \t*) echo >&2 \"Illegal 1\": \"$1\" ; return false ;;\n \tesac\n-\tgrep \"will be replaced by\" \"$2\" | sed -e \"s/\\(.*\\) in [^ ]*$/\\1/\" | uniq >\"$2\".actual\n+\tsed -e \"s/^.* \\([^ ]* will be replaced by [^ ]*\\) .*$/\\1/\" \"$2\" | uniq >\"$2\".actual\n \ttest_cmp \"$2\".expect \"$2\".actual\n }\n \n-- \n2.35.1\n\n"}]}