{"thread":{"id":"52549","subject":"[PATCH 0/1] [Outreachy] [RFC] add: use advise function to display hints","startedAt":"2020-01-02T03:04:05Z","lastAt":"2020-02-05T23:19:05Z","messageCount":26,"participants":["Heba Waly via GitGitGadget","Junio C Hamano","Emily Shaffer","Heba Waly","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"389159","messageId":"pull.508.git.1577934241.gitgitgadget@gmail.com","threadId":"52549","inReplyTo":null,"subject":"[PATCH 0/1] [Outreachy] [RFC] add: use advise function to display hints","fromName":"Heba Waly via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-02T03:04:00Z","receivedAt":"2020-01-02T03:04:05Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"The advise function in advice.c provides a neat and a standard format for\nhint messages, i.e: the text is colored in yellow and the line starts by the\nword \"hint:\".\n\nThis patch suggests using this advise function whenever displaying hints to\nimprove the user experience, as the user's eyes will get used to the format\nand will scan the screen for the yellow hints whenever confused instead of\nreading all the output lines looking for advice.\n\nHeba Waly (1):\n add: use advise function to display hints\n\n builtin/add.c | 4 ++--\n t/t3700-add.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-508%2FHebaWaly%2Fformatting_hints-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-508/HebaWaly/formatting_hints-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/508\n-- \ngitgitgadget\n"},{"id":"389160","messageId":"90608636bf184de76f91e4e04d9e796a021775a0.1577934241.git.gitgitgadget@gmail.com","threadId":"52549","inReplyTo":"pull.508.git.1577934241.gitgitgadget@gmail.com","subject":"[PATCH 1/1] add: use advise function to display hints","fromName":"Heba Waly via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-02T03:04:01Z","receivedAt":"2020-01-02T03:04:06Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"From: Heba Waly \n\nUse the advise function in advice.c to display hints to the users, as\nit provides a neat and a standard format for hint messages, i.e: the\ntext is colored in yellow and the line starts by the word \"hint:\".\n\nSigned-off-by: Heba Waly \n---\n builtin/add.c | 4 ++--\n t/t3700-add.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4c38aff419..eebf8d772b 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfprintf(stderr, _(ignore_error));\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n-\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n+\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n \t\texit_status = 1;\n \t}\n \n@@ -480,7 +480,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (require_pathspec && pathspec.nr == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n+\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex c325167b90..a649805369 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n cat >expect.err <<\\EOF\n The following paths are ignored by one of your .gitignore files:\n ignored-file\n-Use -f if you really want to add them.\n+hint: Use -f if you really want to add them.\n EOF\n cat >expect.out <<\\EOF\n add 'track-this'\n-- \ngitgitgadget\n"},{"id":"389171","messageId":"xmqqpng1eisc.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"90608636bf184de76f91e4e04d9e796a021775a0.1577934241.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-02T19:54:11Z","receivedAt":"2020-01-02T19:54:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Heba Waly via GitGitGadget\" writes:\n\n> From: Heba Waly \n>\n> Use the advise function in advice.c to display hints to the users, as\n> it provides a neat and a standard format for hint messages, i.e: the\n> text is colored in yellow and the line starts by the word \"hint:\".\n\nUse of advise() function is good for giving hints not just due to\nits yellow coloring (which by the way I find not very readable,\nperhaps because I use black ink on white paper). One good thing in\nusing the advise() API is that the messages can also be squelched\nwith advice.* configuration variables.\n\nAnd these two hints in \"git add\" are good chandidates to make\ncustomizable (perhaps with \"advice.addNothing\"), so I tend to agree\nwith you that it makes sense to move these two messages to advise().\nUnfortunately this patch goes only halfway and stops (see below).\n\nIf there are many other places that calls to advise() are made\nwithout getting guarded by the toggles defined in advice.c, we\nshould fix them, I think.\n\n>\n> Signed-off-by: Heba Waly \n> ---\n> builtin/add.c | 4 ++--\n> t/t3700-add.sh | 2 +-\n> 2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 4c38aff419..eebf8d772b 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n> \t\tfprintf(stderr, _(ignore_error));\n> \t\tfor (i = 0; i < dir->ignored_nr; i++)\n> \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> +\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n> \t\texit_status = 1;\n> \t}\n> \n> @@ -480,7 +480,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \n> \tif (require_pathspec && pathspec.nr == 0) {\n> \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> -\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> +\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> \t\treturn 0;\n> \t}\n\nThe final code for the above part would look like:\n\n\t\tif (advice_add_nothing)\n\t\t\tadvise(_(\"Use -f if you really want to add them.\"));\n\t\t...\n\t\tif (advice_add_nothing)\n\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\"));\n\nand then you would\n\n * add defn of advice_add_nothing to advice.h\n * add decl of the same, initialized to 1(true), to advice.c\n * map \"addNothing\" to &advice_add_nothing in advice.c::advice_config[]\n\nto complete the other half of this patch, if the config we choose to\nuse is named \"advice.addNothing\".\n\nBy the way, notice that the single-liner advise() messages do not\nend with LF? This is another difference between printf() family and\nadvise(). advise() cuts its message at LF and prefixes each piece\nwith \"hint:\" but after the final LF there is nothing but NUL, which\nmeans the final LF is optional.\n\nThe warning()/error()/die() family is different from advise() in\nthat they do not chop the incoming message at LF. This behaviour is\nless i18n friendly, and it would be nice to eventually change them\nto behave similarly to advise().\n\nThanks.\n\n \n"},{"id":"389183","messageId":"xmqqzhf5cw69.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"xmqqpng1eisc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-02T22:47:58Z","receivedAt":"2020-01-02T22:48:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Use of advise() function is good for giving hints not just due to\n> its yellow coloring (which by the way I find not very readable,\n> perhaps because I use black ink on white paper). One good thing in\n> using the advise() API is that the messages can also be squelched\n> with advice.* configuration variables.\n\nA side note.\n\nRight now, the advise() API is a bit awkweard to use correctly.\nWhen introducing a new advice message, you would\n\n * come up with advice.frotz configuration variable\n\n * define and declare advice_frotz global variable that defaults to\n true\n\n * sprinkle calls like this:\n\n\tif (advice_frotz)\n\t\tadvise(_(\"helpful message about frotz\"));\n\nI am wondering about two things:\n\n (1) if we can update the API so that the above can be reduced to\n just adding calls like:\n\n\tadvise_ng(\"frotz\", _(\"helpful message about frotz\"));\n\n (2) if such a simplified advise_ng API is a good idea to begin\n with.\n\nThere are a few advantages the current API has, but it cuts both\nways.\n\n - Any new advice toggle MUST be registered to the\n advice.c::advice_config[] table. This table can later be\n extended in the future to allow a list of the toggles to be\n produced at runtime.\n\n This can be seen as an easy mechanism to force programmers to\n keep the list up to date. Or it can also be seen as the source\n of extra work.\n\n - advise() calls can be made without being guarded by any advice.*\n configuration variable. In the overly simplified advise_ng() API\n shown above, we cannot expresss a pattern like this:\n\n\tif (advice_frotz) {\n\t\t... make expensive computation to\n\t\t... come up with values that need to be shown\n\t\t... in the advise() message\n\t\tchar *result = expensive_computation(...);\n\n\t\tadvise(_(\"message %s about frotz\", result));\n\t\tfree(result);\n\t}\n\n without adding another helper function. e.g.\n\n\tif (advise_ng_enabled(\"frotz\")) {\n\t\tchar *result = expensive_computation(...);\n\n\t\t/*\n * advise_ng(\"frotz\", _(\"message %s about frotz\", result));\n * is fine as well, but slightly less efficient as\n * it would involve another call to *_enabled(), so use\n\t\t * the unconditional form of the call\n\t\t */\n\t\tadvise_ng_raw(_(\"message %s about frotz\", result));\n\n\t\tfree(result);\n\t}\n\n"},{"id":"389295","messageId":"20200106230712.GA181522@google.com","threadId":"52549","inReplyTo":"90608636bf184de76f91e4e04d9e796a021775a0.1577934241.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-06T23:07:12Z","receivedAt":"2020-01-06T23:07:21Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jan 02, 2020 at 03:04:01AM +0000, Heba Waly via GitGitGadget wrote:\n> From: Heba Waly \n> \n> @@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n> \t\tfprintf(stderr, _(ignore_error));\n> \t\tfor (i = 0; i < dir->ignored_nr; i++)\n> \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> +\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n\nIn the vein of the rest of your project, for me I'd rather see a\ncopy-pasteable response here:\n\n\"Use 'git add -f \" + name + \"' if you really want to add them.\"\n\nThat is, if you know the name of the file that was being added here, you\ncould provide it so the user can simply copy and go, rather than\nretyping.\n\n\n - Emily\n"},{"id":"389297","messageId":"xmqq36cs89gz.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"20200106230712.GA181522@google.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T23:13:16Z","receivedAt":"2020-01-06T23:13:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> On Thu, Jan 02, 2020 at 03:04:01AM +0000, Heba Waly via GitGitGadget wrote:\n>> From: Heba Waly \n>> \n>> @@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n>> \t\tfprintf(stderr, _(ignore_error));\n>> \t\tfor (i = 0; i < dir->ignored_nr; i++)\n>> \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n>> -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n>> +\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n>\n> In the vein of the rest of your project, for me I'd rather see a\n> copy-pasteable response here:\n>\n> \"Use 'git add -f \" + name + \"' if you really want to add them.\"\n>\n> That is, if you know the name of the file that was being added here, you\n> could provide it so the user can simply copy and go, rather than\n> retyping.\n\nJust being a devil's advocate, but you are opening a can of worms by\nsuggesting so---the path needs to be quoted proporly (and the way to\ndo so may be different depending on the shell in use), for example.\n\n"},{"id":"389298","messageId":"20200106231327.GB181522@google.com","threadId":"52549","inReplyTo":"xmqqpng1eisc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-06T23:13:27Z","receivedAt":"2020-01-06T23:13:34Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jan 02, 2020 at 11:54:11AM -0800, Junio C Hamano wrote:\n> \"Heba Waly via GitGitGadget\" writes:\n> \n> > From: Heba Waly \n> >\n> > Use the advise function in advice.c to display hints to the users, as\n> > it provides a neat and a standard format for hint messages, i.e: the\n> > text is colored in yellow and the line starts by the word \"hint:\".\n> \n> Use of advise() function is good for giving hints not just due to\n> its yellow coloring (which by the way I find not very readable,\n> perhaps because I use black ink on white paper). One good thing in\n> using the advise() API is that the messages can also be squelched\n> with advice.* configuration variables.\n> \n> And these two hints in \"git add\" are good chandidates to make\n> customizable (perhaps with \"advice.addNothing\"), so I tend to agree\n> with you that it makes sense to move these two messages to advise().\n> Unfortunately this patch goes only halfway and stops (see below).\n> \n> If there are many other places that calls to advise() are made\n> without getting guarded by the toggles defined in advice.c, we\n> should fix them, I think.\n\nMaybe this is my C++ habits not dying when they should :) but to me,\nthis begs the question, \"why doesn't advise() check the toggles for me?\"\n\nAre advice messages 1:1 with advice settings? Is there a reason that\nadvise() doesn't look up its corresponding config for itself?\n\n - Emily\n\n> \n> >\n> > Signed-off-by: Heba Waly \n> > ---\n> > builtin/add.c | 4 ++--\n> > t/t3700-add.sh | 2 +-\n> > 2 files changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index 4c38aff419..eebf8d772b 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n> > \t\tfprintf(stderr, _(ignore_error));\n> > \t\tfor (i = 0; i < dir->ignored_nr; i++)\n> > \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> > -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> > +\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n> > \t\texit_status = 1;\n> > \t}\n> > \n> > @@ -480,7 +480,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> > \n> > \tif (require_pathspec && pathspec.nr == 0) {\n> > \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> > -\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> > +\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> > \t\treturn 0;\n> > \t}\n> \n> The final code for the above part would look like:\n> \n> \t\tif (advice_add_nothing)\n> \t\t\tadvise(_(\"Use -f if you really want to add them.\"));\n> \t\t...\n> \t\tif (advice_add_nothing)\n> \t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\"));\n> \n\nHm, I guess this answers my question above about them being 1:1. But I\nsuppose it doesn't necessarily preclude advise() from associating a\nsingle config with multiple advice messages.\n\n - Emily\n"},{"id":"389299","messageId":"xmqqy2uk6uof.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"20200106231327.GB181522@google.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T23:18:08Z","receivedAt":"2020-01-06T23:18:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> Hm, I guess this answers my question above about them being 1:1. But I\n> suppose it doesn't necessarily preclude advise() from associating a\n> single config with multiple advice messages.\n\n... and probably the other message in the thread from me would\nanswer any remaining question you may have, I guess ;-)\n"},{"id":"389321","messageId":"CACg5j24jA1G3b2Efths-dOxPjOPJAM3O5yQfm=K5zFZexbk1eQ@mail.gmail.com","threadId":"52549","inReplyTo":"xmqqpng1eisc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-07T04:19:47Z","receivedAt":"2020-01-07T04:20:02Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Fri, Jan 3, 2020 at 8:54 AM Junio C Hamano wrote:\n>\n> \"Heba Waly via GitGitGadget\" writes:\n>\n> > From: Heba Waly \n> >\n> > Use the advise function in advice.c to display hints to the users, as\n> > it provides a neat and a standard format for hint messages, i.e: the\n> > text is colored in yellow and the line starts by the word \"hint:\".\n>\n> Use of advise() function is good for giving hints not just due to\n> its yellow coloring (which by the way I find not very readable,\n> perhaps because I use black ink on white paper). One good thing in\n> using the advise() API is that the messages can also be squelched\n> with advice.* configuration variables.\n>\n\nGot it, thanks.\n\n> And these two hints in \"git add\" are good chandidates to make\n> customizable (perhaps with \"advice.addNothing\"), so I tend to agree\n> with you that it makes sense to move these two messages to advise().\n> Unfortunately this patch goes only halfway and stops (see below).\n>\n> If there are many other places that calls to advise() are made\n> without getting guarded by the toggles defined in advice.c, we\n> should fix them, I think.\n>\n\nOk, we can address that in a separate patch.\n\n> >\n> > Signed-off-by: Heba Waly \n> > ---\n> > builtin/add.c | 4 ++--\n> > t/t3700-add.sh | 2 +-\n> > 2 files changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index 4c38aff419..eebf8d772b 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -390,7 +390,7 @@ static int add_files(struct dir_struct *dir, int flags)\n> > fprintf(stderr, _(ignore_error));\n> > for (i = 0; i < dir->ignored_nr; i++)\n> > fprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> > - fprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> > + advise(_(\"Use -f if you really want to add them.\\n\"));\n> > exit_status = 1;\n> > }\n> >\n> > @@ -480,7 +480,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> >\n> > if (require_pathspec && pathspec.nr == 0) {\n> > fprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> > - fprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> > + advise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> > return 0;\n> > }\n>\n> The final code for the above part would look like:\n>\n> if (advice_add_nothing)\n> advise(_(\"Use -f if you really want to add them.\"));\n> ...\n> if (advice_add_nothing)\n> advise( _(\"Maybe you wanted to say 'git add .'?\"));\n>\n> and then you would\n>\n> * add defn of advice_add_nothing to advice.h\n> * add decl of the same, initialized to 1(true), to advice.c\n> * map \"addNothing\" to &advice_add_nothing in advice.c::advice_config[]\n>\n> to complete the other half of this patch, if the config we choose to\n> use is named \"advice.addNothing\".\n>\n\nUnderstood.\n\n\n> By the way, notice that the single-liner advise() messages do not\n> end with LF? This is another difference between printf() family and\n> advise(). advise() cuts its message at LF and prefixes each piece\n> with \"hint:\" but after the final LF there is nothing but NUL, which\n> means the final LF is optional.\n>\n> The warning()/error()/die() family is different from advise() in\n> that they do not chop the incoming message at LF. This behaviour is\n> less i18n friendly, and it would be nice to eventually change them\n> to behave similarly to advise().\n>\n\nThank you for the extra tip.\n\n> Thanks.\n>\n>\n\nHeba\n"},{"id":"389348","messageId":"CACg5j27ce5BfR9RKekMEXokvCnXiXzmCVsyEKce+HORe8kL_GQ@mail.gmail.com","threadId":"52549","inReplyTo":"xmqqzhf5cw69.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-07T10:54:04Z","receivedAt":"2020-01-07T10:54:18Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Fri, Jan 3, 2020 at 11:48 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> A side note.\n>\n> Right now, the advise() API is a bit awkweard to use correctly.\n> When introducing a new advice message, you would\n>\n> * come up with advice.frotz configuration variable\n>\n> * define and declare advice_frotz global variable that defaults to\n> true\n>\n> * sprinkle calls like this:\n>\n> if (advice_frotz)\n> advise(_(\"helpful message about frotz\"));\n>\n> I am wondering about two things:\n>\n> (1) if we can update the API so that the above can be reduced to\n> just adding calls like:\n>\n> advise_ng(\"frotz\", _(\"helpful message about frotz\"));\n>\n> (2) if such a simplified advise_ng API is a good idea to begin\n> with.\n>\n\nThat's a valid suggestion, I can investigate that in a new patch, I'd rather\nkeep this one as simple as calling the existing advise function.\n\nThanks,\n\nHeba\n"},{"id":"389393","messageId":"xmqqd0bv6x79.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"CACg5j27ce5BfR9RKekMEXokvCnXiXzmCVsyEKce+HORe8kL_GQ@mail.gmail.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-07T16:35:54Z","receivedAt":"2020-01-07T16:35:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heba Waly writes:\n\n> On Fri, Jan 3, 2020 at 11:48 AM Junio C Hamano wrote:\n>>\n>> Junio C Hamano writes:\n>>\n>> A side note.\n>>\n>> Right now, the advise() API is a bit awkweard to use correctly.\n>> When introducing a new advice message, you would\n>>\n>> * come up with advice.frotz configuration variable\n>>\n>> * define and declare advice_frotz global variable that defaults to\n>> true\n>>\n>> * sprinkle calls like this:\n>>\n>> if (advice_frotz)\n>> advise(_(\"helpful message about frotz\"));\n>>\n>> I am wondering about two things:\n>>\n>> (1) if we can update the API so that the above can be reduced to\n>> just adding calls like:\n>>\n>> advise_ng(\"frotz\", _(\"helpful message about frotz\"));\n>>\n>> (2) if such a simplified advise_ng API is a good idea to begin\n>> with.\n>>\n>\n> That's a valid suggestion, I can investigate that in a new patch, I'd rather\n> keep this one as simple as calling the existing advise function.\n\nYeah, the side note wasn't even a suggestion for improving _this_\ntopic, nor even specifically addressed to you. Let's stay focused.\n\nThanks.\n\n"},{"id":"389433","messageId":"pull.508.v2.git.1578438752.gitgitgadget@gmail.com","threadId":"52549","inReplyTo":"pull.508.git.1577934241.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] [Outreachy] add: use advise function to display hints","fromName":"Heba Waly via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-07T23:12:31Z","receivedAt":"2020-01-07T23:12:36Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"The advise function in advice.c provides a neat and a standard format for\nhint messages, i.e: the text is colored in yellow and the line starts by the\nword \"hint:\". Also this will allow us to control the hint messages based on\nadvice.* configuration variables.\n\nThis patch suggests using this advise function whenever displaying hints to\nimprove the user experience, as the user's eyes will get used to the format\nand will scan the screen for the yellow hints whenever confused instead of\nreading all the output lines looking for advice.\n\nHeba Waly (1):\n add: use advise function to display hints\n\n advice.c | 2 ++\n advice.h | 1 +\n builtin/add.c | 6 ++++--\n t/t3700-add.sh | 2 +-\n 4 files changed, 8 insertions(+), 3 deletions(-)\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-508%2FHebaWaly%2Fformatting_hints-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-508/HebaWaly/formatting_hints-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/508\n\nRange-diff vs v1:\n\n 1: 90608636bf ! 1: 9f9febd3f4 add: use advise function to display hints\n @@ -6,8 +6,43 @@\n it provides a neat and a standard format for hint messages, i.e: the\n text is colored in yellow and the line starts by the word \"hint:\".\n \n + Also this will enable us to control the messages using advice.*\n + configuration variables.\n +\n Signed-off-by: Heba Waly \n \n + diff --git a/advice.c b/advice.c\n + --- a/advice.c\n + +++ b/advice.c\n +@@\n + int advice_checkout_ambiguous_remote_branch_name = 1;\n + int advice_nested_tag = 1;\n + int advice_submodule_alternate_error_strategy_die = 1;\n ++int advice_add_nothing = 1;\n + \n + static int advice_use_color = -1;\n + static char advice_colors[][COLOR_MAXLEN] = {\n +@@\n + \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n + \t{ \"nestedTag\", &advice_nested_tag },\n + \t{ \"submoduleAlternateErrorStrategyDie\", &advice_submodule_alternate_error_strategy_die },\n ++\t{ \"addNothing\", &advice_add_nothing },\n + \n + \t/* make this an alias for backward compatibility */\n + \t{ \"pushNonFastForward\", &advice_push_update_rejected }\n +\n + diff --git a/advice.h b/advice.h\n + --- a/advice.h\n + +++ b/advice.h\n +@@\n + extern int advice_checkout_ambiguous_remote_branch_name;\n + extern int advice_nested_tag;\n + extern int advice_submodule_alternate_error_strategy_die;\n ++extern int advice_add_nothing;\n + \n + int git_default_advice_config(const char *var, const char *value);\n + __attribute__((format (printf, 1, 2)))\n +\n diff --git a/builtin/add.c b/builtin/add.c\n --- a/builtin/add.c\n +++ b/builtin/add.c\n @@ -16,7 +51,8 @@\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n -+\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n ++\t\tif (advice_add_nothing)\n ++\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n \t\texit_status = 1;\n \t}\n \n @@ -25,7 +61,8 @@\n \tif (require_pathspec && pathspec.nr == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n -\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n -+\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n ++\t\tif (advice_add_nothing)\n ++\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n \t\treturn 0;\n \t}\n \n\n-- \ngitgitgadget\n"},{"id":"389434","messageId":"9f9febd3f4f7f82178fceac98fcc91cb28a1b3b9.1578438752.git.gitgitgadget@gmail.com","threadId":"52549","inReplyTo":"pull.508.v2.git.1578438752.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] add: use advise function to display hints","fromName":"Heba Waly via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-07T23:12:32Z","receivedAt":"2020-01-07T23:12:37Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"From: Heba Waly \n\nUse the advise function in advice.c to display hints to the users, as\nit provides a neat and a standard format for hint messages, i.e: the\ntext is colored in yellow and the line starts by the word \"hint:\".\n\nAlso this will enable us to control the messages using advice.*\nconfiguration variables.\n\nSigned-off-by: Heba Waly \n---\n advice.c | 2 ++\n advice.h | 1 +\n builtin/add.c | 6 ++++--\n t/t3700-add.sh | 2 +-\n 4 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 249c60dcf3..098ac0abea 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -31,6 +31,7 @@ int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n int advice_nested_tag = 1;\n int advice_submodule_alternate_error_strategy_die = 1;\n+int advice_add_nothing = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -91,6 +92,7 @@ static struct {\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n \t{ \"nestedTag\", &advice_nested_tag },\n \t{ \"submoduleAlternateErrorStrategyDie\", &advice_submodule_alternate_error_strategy_die },\n+\t{ \"addNothing\", &advice_add_nothing },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex b706780614..83287b0594 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -31,6 +31,7 @@ extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n extern int advice_nested_tag;\n extern int advice_submodule_alternate_error_strategy_die;\n+extern int advice_add_nothing;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4c38aff419..57b3186f69 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -390,7 +390,8 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfprintf(stderr, _(ignore_error));\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n-\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n+\t\tif (advice_add_nothing)\n+\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n \t\texit_status = 1;\n \t}\n \n@@ -480,7 +481,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (require_pathspec && pathspec.nr == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n+\t\tif (advice_add_nothing)\n+\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex c325167b90..a649805369 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n cat >expect.err <<\\EOF\n The following paths are ignored by one of your .gitignore files:\n ignored-file\n-Use -f if you really want to add them.\n+hint: Use -f if you really want to add them.\n EOF\n cat >expect.out <<\\EOF\n add 'track-this'\n-- \ngitgitgadget\n"},{"id":"389435","messageId":"CACg5j250nynzUwjfn6zOGt2-RNJxWYry6ycbm9dBUCTFo7MJ3w@mail.gmail.com","threadId":"52549","inReplyTo":"xmqqd0bv6x79.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] add: use advise function to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-07T23:32:54Z","receivedAt":"2020-01-07T23:33:09Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Wed, Jan 8, 2020 at 5:35 AM Junio C Hamano wrote:\n>\n> Heba Waly writes:\n>\n> > On Fri, Jan 3, 2020 at 11:48 AM Junio C Hamano wrote:\n> >>\n> >> Junio C Hamano writes:\n> >>\n> > That's a valid suggestion, I can investigate that in a new patch, I'd rather\n> > keep this one as simple as calling the existing advise function.\n>\n> Yeah, the side note wasn't even a suggestion for improving _this_\n> topic, nor even specifically addressed to you. Let's stay focused.\n>\n\nSending out this patch, not only do I want the community's say in using advise()\nin add.c but using it in general in more locations where hints are\ndisplayed using\nprintf() or any other similar function. So when you pointed out that\nit's not supposed\nto be used without checking the corresponding configuration variable,\nI had a similar\nthought to yours, that it can be improved.\nAccordingly I might be interested in looking in to this once I finish\nwhat I have in hand.\n\nThanks,\n\nHeba\n"},{"id":"390597","messageId":"20200127235210.GC233139@google.com","threadId":"52549","inReplyTo":"9f9febd3f4f7f82178fceac98fcc91cb28a1b3b9.1578438752.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/1] add: use advise function to display hints","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-27T23:52:10Z","receivedAt":"2020-01-27T23:52:19Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Tue, Jan 07, 2020 at 11:12:32PM +0000, Heba Waly via GitGitGadget wrote:\n> From: Heba Waly \n> \n> Use the advise function in advice.c to display hints to the users, as\n> it provides a neat and a standard format for hint messages, i.e: the\n> text is colored in yellow and the line starts by the word \"hint:\".\n> \n> Also this will enable us to control the messages using advice.*\n> configuration variables.\n\nLooks like this slipped through the cracks over the holidays. Sorry! :)\n\n> \n> Signed-off-by: Heba Waly \n> ---\n> advice.c | 2 ++\n> advice.h | 1 +\n> builtin/add.c | 6 ++++--\n> t/t3700-add.sh | 2 +-\n> 4 files changed, 8 insertions(+), 3 deletions(-)\n> \n> diff --git a/advice.c b/advice.c\n> index 249c60dcf3..098ac0abea 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -31,6 +31,7 @@ int advice_graft_file_deprecated = 1;\n> int advice_checkout_ambiguous_remote_branch_name = 1;\n> int advice_nested_tag = 1;\n> int advice_submodule_alternate_error_strategy_die = 1;\n> +int advice_add_nothing = 1;\n\nHere's the global advice setting we can look at.\n\n> \n> static int advice_use_color = -1;\n> static char advice_colors[][COLOR_MAXLEN] = {\n> @@ -91,6 +92,7 @@ static struct {\n> \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n> \t{ \"nestedTag\", &advice_nested_tag },\n> \t{ \"submoduleAlternateErrorStrategyDie\", &advice_submodule_alternate_error_strategy_die },\n> +\t{ \"addNothing\", &advice_add_nothing },\n\nHere's the name of the advice config, e.g. advice.addNothing.\n\nHmm, I wonder if addNothing really makes sense/is understandable when\nI'm configuring? I see two cases you're addressing; first, adding an\nignored file (\"Use -f if you really want to add\") - which \"addNothing\"\ndoesn't really make sense for - and second, \"add\" with nothing\nspecified (\"did you mean 'git add .'\"), where \"addNothing\" makes sense\nin context. Out of context though, perhaps \"hint.addIgnoredFile\" and\n\"hint.addEmptyPathspec\" make more sense? Of course naming is one of the\ntwo hardest problems in computer science (next to race conditions and\noff-by-one errors) so probably someone else can suggest a better name :)\n\n> \n> \t/* make this an alias for backward compatibility */\n> \t{ \"pushNonFastForward\", &advice_push_update_rejected }\n> diff --git a/advice.h b/advice.h\n> index b706780614..83287b0594 100644\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -31,6 +31,7 @@ extern int advice_graft_file_deprecated;\n> extern int advice_checkout_ambiguous_remote_branch_name;\n> extern int advice_nested_tag;\n> extern int advice_submodule_alternate_error_strategy_die;\n> +extern int advice_add_nothing;\n> \n> int git_default_advice_config(const char *var, const char *value);\n> __attribute__((format (printf, 1, 2)))\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 4c38aff419..57b3186f69 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -390,7 +390,8 @@ static int add_files(struct dir_struct *dir, int flags)\n> \t\tfprintf(stderr, _(ignore_error));\n> \t\tfor (i = 0; i < dir->ignored_nr; i++)\n> \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> +\t\tif (advice_add_nothing)\n> +\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n\nHere's where we add the guard, and use the new config.\n\nAs mentioned earlier, I'm not sure that tying this advice to the same\nconfig as the next one you change really makes sense.\n\nNitwise, it's somewhat common for advice hints to also tell you how to\ndisable them; see sha1-name.c:get_oid_basic's 'object_name_msg' for an\nexample.\n\n> \t\texit_status = 1;\n> \t}\n> \n> @@ -480,7 +481,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \n> \tif (require_pathspec && pathspec.nr == 0) {\n> \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> -\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> +\t\tif (advice_add_nothing)\n> +\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n\nSame nit as above.\n\n> \t\treturn 0;\n> \t}\n> \n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index c325167b90..a649805369 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n> cat >expect.err <<\\EOF\n> The following paths are ignored by one of your .gitignore files:\n> ignored-file\n> -Use -f if you really want to add them.\n> +hint: Use -f if you really want to add them.\n> EOF\n> cat >expect.out <<\\EOF\n> add 'track-this'\n> -- \n> gitgitgadget\n\nFinally, you'd better update Documentation/config/advice.txt too.\n"},{"id":"390598","messageId":"20200128000047.176372-1-jonathantanmy@google.com","threadId":"52549","inReplyTo":"9f9febd3f4f7f82178fceac98fcc91cb28a1b3b9.1578438752.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/1] add: use advise function to display hints","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-01-28T00:00:47Z","receivedAt":"2020-01-28T00:00:53Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> From: Heba Waly \n> \n> Use the advise function in advice.c to display hints to the users, as\n> it provides a neat and a standard format for hint messages, i.e: the\n> text is colored in yellow and the line starts by the word \"hint:\".\n> \n> Also this will enable us to control the messages using advice.*\n> configuration variables.\n\nFirstly, sorry for getting back to this so late.\n\nAs written, this gives me the impression that advise() is what enables\nus to control the messages using configuration variables, but that's not\ntrue - that's done by a separate mechanism in advise.c and .h.\nParaphrasing what Junio wrote [1], the commit message might be better\nwritten as:\n\n In the \"add\" command, use the advice API instead of fprintf() for the\n hint shown when nothing was added. Thus, this hint message follows the\n standard hint message format, and its visibility is made configurable.\n\n(Note that I mentioned the \"add\" command and called it the advice API\ninstead of the advise() function.)\n\n(Feel free to use this or write your own.)\n\n[1] https://lore.kernel.org/git/xmqqpng1eisc.fsf@gitster-ct.c.googlers.com/\n\n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index c325167b90..a649805369 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n> cat >expect.err <<\\EOF\n> The following paths are ignored by one of your .gitignore files:\n> ignored-file\n> -Use -f if you really want to add them.\n> +hint: Use -f if you really want to add them.\n> EOF\n> cat >expect.out <<\\EOF\n> add 'track-this'\n\nAlso add a test that checks what happens if advice.addNothing is set.\n(It seems that we generally don't test what happens when advice is\nsuppressed. If we consider solely this patch, I'm on the fence of the\nusefulness of this test, but if we plan to refactor the advise()\nfunction to take care of checking the config variable itself, for\nexample, then we will need such a test anyway, so I think we might as\nwell include at least one such advice test now.)\n"},{"id":"390678","messageId":"CACg5j26DEXuxwqRYHi5UOBUpRwsu_2A9LwgyKq4qB9wxqasD7g@mail.gmail.com","threadId":"52549","inReplyTo":"20200127235210.GC233139@google.com","subject":"Re: [PATCH v2 1/1] add: use advise function to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-29T01:09:21Z","receivedAt":"2020-01-29T01:09:36Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Tue, Jan 28, 2020 at 12:52 PM Emily Shaffer wrote:\n>\n> Hmm, I wonder if addNothing really makes sense/is understandable when\n> I'm configuring? I see two cases you're addressing; first, adding an\n> ignored file (\"Use -f if you really want to add\") - which \"addNothing\"\n> doesn't really make sense for - and second, \"add\" with nothing\n> specified (\"did you mean 'git add .'\"), where \"addNothing\" makes sense\n> in context. Out of context though, perhaps \"hint.addIgnoredFile\" and\n> \"hint.addEmptyPathspec\" make more sense? Of course naming is one of the\n> two hardest problems in computer science (next to race conditions and\n> off-by-one errors) so probably someone else can suggest a better name :)\n>\n\nI agree, as this patch was my first interaction with the advice\nlibrary, but now after many discussions on different threads it makes\nmore sense to add two config variables for the two messages.\n\n>\n> As mentioned earlier, I'm not sure that tying this advice to the same\n> config as the next one you change really makes sense.\n>\n> Nitwise, it's somewhat common for advice hints to also tell you how to\n> disable them; see sha1-name.c:get_oid_basic's 'object_name_msg' for an\n> example.\n>\n\nI can see that this was followed in only three locations around the\ncode base, which means that not telling the user how to disable the\nhint is more common.\nInitially I tended to think of it as noise as I suspect the user will\nignore this extra line about disabling the message more often. But\nafter taking a second look at Documentation/config/advice.txt I\nrealized how hard it will be for the user to find the corresponding\nconfiguration variable to the message that he/she would like to turn\noff,\nspecially when the list is getting longer. So seems like displaying\nthe extra note will make the user's life easier *when* s/he wants to\nturn it off.\n\n> > exit_status = 1;\n> > }\n> >\n> > @@ -480,7 +481,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> >\n> > if (require_pathspec && pathspec.nr == 0) {\n> > fprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> > - fprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n> > + if (advice_add_nothing)\n> > + advise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n>\n> Same nit as above.\n>\n> > return 0;\n> > }\n> >\n> > diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> > index c325167b90..a649805369 100755\n> > --- a/t/t3700-add.sh\n> > +++ b/t/t3700-add.sh\n> > @@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n> > cat >expect.err <<\\EOF\n> > The following paths are ignored by one of your .gitignore files:\n> > ignored-file\n> > -Use -f if you really want to add them.\n> > +hint: Use -f if you really want to add them.\n> > EOF\n> > cat >expect.out <<\\EOF\n> > add 'track-this'\n> > --\n> > gitgitgadget\n>\n> Finally, you'd better update Documentation/config/advice.txt too.\n\nYeah, got that on my todo list :)\n\nThanks,\nHeba\n"},{"id":"390680","messageId":"CACg5j25fi0nSMe2nH+iJ184ouk2VsrBWSZmEv-C_PrY8qfq4Sw@mail.gmail.com","threadId":"52549","inReplyTo":"20200128000047.176372-1-jonathantanmy@google.com","subject":"Re: [PATCH v2 1/1] add: use advise function to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-29T02:04:17Z","receivedAt":"2020-01-29T02:09:07Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Tue, Jan 28, 2020 at 1:00 PM Jonathan Tan wrote:\n>\n> > From: Heba Waly \n> >\n> > Use the advise function in advice.c to display hints to the users, as\n> > it provides a neat and a standard format for hint messages, i.e: the\n> > text is colored in yellow and the line starts by the word \"hint:\".\n> >\n> > Also this will enable us to control the messages using advice.*\n> > configuration variables.\n>\n> Firstly, sorry for getting back to this so late.\n>\n> As written, this gives me the impression that advise() is what enables\n> us to control the messages using configuration variables, but that's not\n> true - that's done by a separate mechanism in advise.c and .h.\n> Paraphrasing what Junio wrote [1], the commit message might be better\n> written as:\n>\n> In the \"add\" command, use the advice API instead of fprintf() for the\n> hint shown when nothing was added. Thus, this hint message follows the\n> standard hint message format, and its visibility is made configurable.\n>\n> (Note that I mentioned the \"add\" command and called it the advice API\n> instead of the advise() function.)\n>\n\nThat makes sense.\n\n> (Feel free to use this or write your own.)\n>\n> [1] https://lore.kernel.org/git/xmqqpng1eisc.fsf@gitster-ct.c.googlers.com/\n>\n> > diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> > index c325167b90..a649805369 100755\n> > --- a/t/t3700-add.sh\n> > +++ b/t/t3700-add.sh\n> > @@ -326,7 +326,7 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n> > cat >expect.err <<\\EOF\n> > The following paths are ignored by one of your .gitignore files:\n> > ignored-file\n> > -Use -f if you really want to add them.\n> > +hint: Use -f if you really want to add them.\n> > EOF\n> > cat >expect.out <<\\EOF\n> > add 'track-this'\n>\n> Also add a test that checks what happens if advice.addNothing is set.\n> (It seems that we generally don't test what happens when advice is\n> suppressed. If we consider solely this patch, I'm on the fence of the\n> usefulness of this test, but if we plan to refactor the advise()\n> function to take care of checking the config variable itself, for\n> example, then we will need such a test anyway, so I think we might as\n> well include at least one such advice test now.)\n\nI'm tempted to say let's worry about it when refactoring advise(),\nmaybe then we'll\nfind a more suitable place for this test. as it'll be advice-related,\nnot caller-related.\n\nThanks,\nHeba\n"},{"id":"390760","messageId":"pull.508.v3.git.1580346702203.gitgitgadget@gmail.com","threadId":"52549","inReplyTo":"pull.508.v2.git.1578438752.gitgitgadget@gmail.com","subject":"[PATCH v3] add: use advice API to display hints","fromName":"Heba Waly via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-30T01:11:41Z","receivedAt":"2020-01-30T01:11:47Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"From: Heba Waly \n\nIn the \"add\" command, use the advice API to display hints to users,\nas it provides a neat and a standard format for hint messages, and\nthe message visibility will be configurable.\n\nSigned-off-by: Heba Waly \n---\n [Outreachy] add: use advise API to display hints\n \n In the \"add\" command, use the advice API to display hints to users, as\n it provides a neat and a standard format for hint messages, and the\n message visibility will be configurable.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-508%2FHebaWaly%2Fformatting_hints-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-508/HebaWaly/formatting_hints-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/508\n\nRange-diff vs v2:\n\n 1: 9f9febd3f4 ! 1: 410a66953d add: use advise function to display hints\n @@ -1,16 +1,28 @@\n Author: Heba Waly \n \n - add: use advise function to display hints\n + add: use advice API to display hints\n \n - Use the advise function in advice.c to display hints to the users, as\n - it provides a neat and a standard format for hint messages, i.e: the\n - text is colored in yellow and the line starts by the word \"hint:\".\n -\n - Also this will enable us to control the messages using advice.*\n - configuration variables.\n + In the \"add\" command, use the advice API to display hints to users,\n + as it provides a neat and a standard format for hint messages, and\n + the message visibility will be configurable.\n \n Signed-off-by: Heba Waly \n \n + diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n + --- a/Documentation/config/advice.txt\n + +++ b/Documentation/config/advice.txt\n +@@\n + \tsubmoduleAlternateErrorStrategyDie:\n + \t\tAdvice shown when a submodule.alternateErrorStrategy option\n + \t\tconfigured to \"die\" causes a fatal error.\n ++\taddIgnoredFile::\n ++\t\tAdvice shown if a user attempts to add an ignored file to\n ++\t\tthe index.\n ++\taddEmptyPathspec::\n ++\t\tAdvice shown if a user runs the add command without providing\n ++\t\tthe pathspec parameter.\n + --\n +\n diff --git a/advice.c b/advice.c\n --- a/advice.c\n +++ b/advice.c\n @@ -18,7 +30,8 @@\n int advice_checkout_ambiguous_remote_branch_name = 1;\n int advice_nested_tag = 1;\n int advice_submodule_alternate_error_strategy_die = 1;\n -+int advice_add_nothing = 1;\n ++int advice_add_ignored_file = 1;\n ++int advice_add_empty_pathspec = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n @@ -26,7 +39,8 @@\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n \t{ \"nestedTag\", &advice_nested_tag },\n \t{ \"submoduleAlternateErrorStrategyDie\", &advice_submodule_alternate_error_strategy_die },\n -+\t{ \"addNothing\", &advice_add_nothing },\n ++\t{ \"addIgnoredFile\", &advice_add_ignored_file },\n ++\t{ \"addEmptyPathspec\", &advice_add_empty_pathspec },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\n @@ -38,7 +52,8 @@\n extern int advice_checkout_ambiguous_remote_branch_name;\n extern int advice_nested_tag;\n extern int advice_submodule_alternate_error_strategy_die;\n -+extern int advice_add_nothing;\n ++extern int advice_add_ignored_file;\n ++extern int advice_add_empty_pathspec;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\n @@ -51,8 +66,10 @@\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n -\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n -+\t\tif (advice_add_nothing)\n -+\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"));\n ++\t\tif (advice_add_ignored_file)\n ++\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"\n ++\t\t\t\t \"Turn this message off by running\\n\"\n ++\t\t\t\t \"\\\"git config advice.addIgnoredFile false\\\"\"));\n \t\texit_status = 1;\n \t}\n \n @@ -61,8 +78,10 @@\n \tif (require_pathspec && pathspec.nr == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n -\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n -+\t\tif (advice_add_nothing)\n -+\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"));\n ++\t\tif (advice_add_empty_pathspec)\n ++\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"\n ++\t\t\t\t \"Turn this message off by running\\n\"\n ++\t\t\t\t \"\\\"git config advice.addEmptyPathspec false\\\"\"));\n \t\treturn 0;\n \t}\n \n @@ -76,6 +95,8 @@\n ignored-file\n -Use -f if you really want to add them.\n +hint: Use -f if you really want to add them.\n ++hint: Turn this message off by running\n ++hint: \"git config advice.addIgnoredFile false\"\n EOF\n cat >expect.out <<\\EOF\n add 'track-this'\n\n\n Documentation/config/advice.txt | 6 ++++++\n advice.c | 4 ++++\n advice.h | 2 ++\n builtin/add.c | 10 ++++++++--\n t/t3700-add.sh | 4 +++-\n 5 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex d4e698cd3f..a72615c68d 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -110,4 +110,10 @@ advice.*::\n \tsubmoduleAlternateErrorStrategyDie:\n \t\tAdvice shown when a submodule.alternateErrorStrategy option\n \t\tconfigured to \"die\" causes a fatal error.\n+\taddIgnoredFile::\n+\t\tAdvice shown if a user attempts to add an ignored file to\n+\t\tthe index.\n+\taddEmptyPathspec::\n+\t\tAdvice shown if a user runs the add command without providing\n+\t\tthe pathspec parameter.\n --\ndiff --git a/advice.c b/advice.c\nindex 249c60dcf3..97f3f981b4 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -31,6 +31,8 @@ int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n int advice_nested_tag = 1;\n int advice_submodule_alternate_error_strategy_die = 1;\n+int advice_add_ignored_file = 1;\n+int advice_add_empty_pathspec = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -91,6 +93,8 @@ static struct {\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n \t{ \"nestedTag\", &advice_nested_tag },\n \t{ \"submoduleAlternateErrorStrategyDie\", &advice_submodule_alternate_error_strategy_die },\n+\t{ \"addIgnoredFile\", &advice_add_ignored_file },\n+\t{ \"addEmptyPathspec\", &advice_add_empty_pathspec },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex b706780614..0e6e58d9f8 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -31,6 +31,8 @@ extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n extern int advice_nested_tag;\n extern int advice_submodule_alternate_error_strategy_die;\n+extern int advice_add_ignored_file;\n+extern int advice_add_empty_pathspec;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4c38aff419..37b6cbac53 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -390,7 +390,10 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfprintf(stderr, _(ignore_error));\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n-\t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n+\t\tif (advice_add_ignored_file)\n+\t\t\tadvise(_(\"Use -f if you really want to add them.\\n\"\n+\t\t\t\t \"Turn this message off by running\\n\"\n+\t\t\t\t \"\\\"git config advice.addIgnoredFile false\\\"\"));\n \t\texit_status = 1;\n \t}\n \n@@ -480,7 +483,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (require_pathspec && pathspec.nr == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n+\t\tif (advice_add_empty_pathspec)\n+\t\t\tadvise( _(\"Maybe you wanted to say 'git add .'?\\n\"\n+\t\t\t\t \"Turn this message off by running\\n\"\n+\t\t\t\t \"\\\"git config advice.addEmptyPathspec false\\\"\"));\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex c325167b90..88bc799807 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -326,7 +326,9 @@ test_expect_success 'git add --dry-run of an existing file output' \"\n cat >expect.err <<\\EOF\n The following paths are ignored by one of your .gitignore files:\n ignored-file\n-Use -f if you really want to add them.\n+hint: Use -f if you really want to add them.\n+hint: Turn this message off by running\n+hint: \"git config advice.addIgnoredFile false\"\n EOF\n cat >expect.out <<\\EOF\n add 'track-this'\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\n-- \ngitgitgadget\n"},{"id":"390848","messageId":"xmqqimksbo73.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"pull.508.v3.git.1580346702203.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-30T21:59:28Z","receivedAt":"2020-01-30T21:59:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Heba Waly via GitGitGadget\" writes:\n\n> From: Heba Waly \n>\n> In the \"add\" command, use the advice API to display hints to users,\n> as it provides a neat and a standard format for hint messages, and\n> the message visibility will be configurable.\n>\n> Signed-off-by: Heba Waly \n> ---\n> [Outreachy] add: use advise API to display hints\n> \n> In the \"add\" command, use the advice API to display hints to users, as\n> it provides a neat and a standard format for hint messages, and the\n> message visibility will be configurable.\n\nThe topic has been in 'next' for the past week or so already. If we\nneed to make further changes, please do so incrementally.\n\nThanks.\n"},{"id":"390899","messageId":"CACg5j27pTKuhZpZtgNUDNEkhG0+tGx5O=LJCr5E8+2q8v6Zu1w@mail.gmail.com","threadId":"52549","inReplyTo":"xmqqimksbo73.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-01-31T11:16:31Z","receivedAt":"2020-01-31T11:16:45Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Fri, Jan 31, 2020 at 10:59 AM Junio C Hamano wrote:\n>\n> \"Heba Waly via GitGitGadget\" writes:\n>\n> > From: Heba Waly \n> >\n> > In the \"add\" command, use the advice API to display hints to users,\n> > as it provides a neat and a standard format for hint messages, and\n> > the message visibility will be configurable.\n> >\n> > Signed-off-by: Heba Waly \n> > ---\n> > [Outreachy] add: use advise API to display hints\n> >\n> > In the \"add\" command, use the advice API to display hints to users, as\n> > it provides a neat and a standard format for hint messages, and the\n> > message visibility will be configurable.\n>\n> The topic has been in 'next' for the past week or so already. If we\n> need to make further changes, please do so incrementally.\n>\n\nWill do, thanks.\n\n> Thanks.\n"},{"id":"391196","messageId":"xmqq7e10hgwn.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"CACg5j27pTKuhZpZtgNUDNEkhG0+tGx5O=LJCr5E8+2q8v6Zu1w@mail.gmail.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-05T21:18:32Z","receivedAt":"2020-02-05T21:18:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heba Waly writes:\n\n> On Fri, Jan 31, 2020 at 10:59 AM Junio C Hamano wrote:\n>>\n>> \"Heba Waly via GitGitGadget\" writes:\n>>\n>> > From: Heba Waly \n>> >\n>> > In the \"add\" command, use the advice API to display hints to users,\n>> > as it provides a neat and a standard format for hint messages, and\n>> > the message visibility will be configurable.\n>> >\n>> > Signed-off-by: Heba Waly \n>> > ---\n>> > [Outreachy] add: use advise API to display hints\n>> >\n>> > In the \"add\" command, use the advice API to display hints to users, as\n>> > it provides a neat and a standard format for hint messages, and the\n>> > message visibility will be configurable.\n>>\n>> The topic has been in 'next' for the past week or so already. If we\n>> need to make further changes, please do so incrementally.\n>\n> Will do, thanks.\n\nI was reviewing the draft of the \"What's cooking\" report and noticed\nthat this update is not there---did I miss one?\n\nThanks.\n"},{"id":"391198","messageId":"CACg5j252=wKyh7Ar9vxTwxdYXgkjNvbMA=bJCKOc6UZRJfJmUg@mail.gmail.com","threadId":"52549","inReplyTo":"xmqq7e10hgwn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-02-05T22:05:33Z","receivedAt":"2020-02-05T22:05:46Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"No, I agreed with my mentors to wait on this update until that branch\nis merged in master.\nSo no need to worry about it.\n\nThanks,\nHeba\n\nOn Thu, Feb 6, 2020 at 10:18 AM Junio C Hamano wrote:\n>\n> Heba Waly writes:\n>\n> > On Fri, Jan 31, 2020 at 10:59 AM Junio C Hamano wrote:\n> >>\n> >> \"Heba Waly via GitGitGadget\" writes:\n> >>\n> >> > From: Heba Waly \n> >> >\n> >> > In the \"add\" command, use the advice API to display hints to users,\n> >> > as it provides a neat and a standard format for hint messages, and\n> >> > the message visibility will be configurable.\n> >> >\n> >> > Signed-off-by: Heba Waly \n> >> > ---\n> >> > [Outreachy] add: use advise API to display hints\n> >> >\n> >> > In the \"add\" command, use the advice API to display hints to users, as\n> >> > it provides a neat and a standard format for hint messages, and the\n> >> > message visibility will be configurable.\n> >>\n> >> The topic has been in 'next' for the past week or so already. If we\n> >> need to make further changes, please do so incrementally.\n> >\n> > Will do, thanks.\n>\n> I was reviewing the draft of the \"What's cooking\" report and noticed\n> that this update is not there---did I miss one?\n>\n> Thanks.\n"},{"id":"391199","messageId":"xmqqy2tgfzk1.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"CACg5j252=wKyh7Ar9vxTwxdYXgkjNvbMA=bJCKOc6UZRJfJmUg@mail.gmail.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-05T22:18:38Z","receivedAt":"2020-02-05T22:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heba Waly writes:\n\n> No, I agreed with my mentors to wait on this update until that branch\n> is merged in master.\n\nThe users will first has to set advise.addnothing and then later has\nto set something different if you do so, no?\n\nI do not think that is a good decision, and I am not happy to see\npeople making such a decision that would hurt our users off list.\n\n> So no need to worry about it.\n\nYes, I do have to worry about our users.\n"},{"id":"391213","messageId":"CACg5j25YpyRbxmYccZZiG9m5a0DKm0RMD3ypy2JzhA-bmgB_9w@mail.gmail.com","threadId":"52549","inReplyTo":"xmqqy2tgfzk1.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Heba Waly","fromEmail":"heba.waly@gmail.com","sentAt":"2020-02-05T23:05:03Z","receivedAt":"2020-02-05T23:05:17Z","isPatch":true,"sender":{"key":"heba.waly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1539076?v=4"},"body":"On Thu, Feb 6, 2020 at 11:18 AM Junio C Hamano wrote:\n>\n> Heba Waly writes:\n>\n> > No, I agreed with my mentors to wait on this update until that branch\n> > is merged in master.\n>\n> The users will first has to set advise.addnothing and then later has\n> to set something different if you do so, no?\n>\n\nYou're right, I missed that point, will send an update based on the\npickup branch shortly.\n\nThanks,\nHeba\n"},{"id":"391214","messageId":"xmqqtv44fwrj.fsf@gitster-ct.c.googlers.com","threadId":"52549","inReplyTo":"CACg5j25YpyRbxmYccZZiG9m5a0DKm0RMD3ypy2JzhA-bmgB_9w@mail.gmail.com","subject":"Re: [PATCH v3] add: use advice API to display hints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-05T23:18:56Z","receivedAt":"2020-02-05T23:19:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heba Waly writes:\n\n> On Thu, Feb 6, 2020 at 11:18 AM Junio C Hamano wrote:\n>>\n>> Heba Waly writes:\n>>\n>> > No, I agreed with my mentors to wait on this update until that branch\n>> > is merged in master.\n>>\n>> The users will first has to set advise.addnothing and then later has\n>> to set something different if you do so, no?\n>>\n>\n> You're right, I missed that point, will send an update based on the\n> pickup branch shortly.\n\nThanks. \n\nI'll keep the topic in 'next' while letting some handful of other\ntopics graduate to 'master' in today's integration cycle. Hopefully\nit can join 'master' shortly.\\\n\n"}]}