{"thread":{"id":"41882","subject":"[PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","startedAt":"2016-03-31T09:25:27Z","lastAt":"2016-04-06T20:05:19Z","messageCount":9,"participants":["Elena Petrashen","Matthieu Moy","Remi Galan Alfonso","Junio C Hamano","elena petrashen"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"282300","messageId":"1459416327-795-1-git-send-email-elena.petrashen@gmail.com","threadId":"41882","inReplyTo":null,"subject":"[PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Elena Petrashen","fromEmail":"elena.petrashen@gmail.com","sentAt":"2016-03-31T09:25:27Z","receivedAt":"2016-03-31T09:25:27Z","isPatch":true,"sender":{"key":"elena.petrashen@gmail.com","avatar":"https://gravatar.com/avatar/a2fd27bdf84eb2b7fc7b034a358aa6777fe86d5a9ac9339f122aea2b6b7689d4?d=mp&s=160"},"body":"Signed-off-by: Elena Petrashen \n---\nThis micro-patch is meant to allow “-“ as a short-hand for\n“@{-1} for branch -d (Cf. $gmane/230828). Based on feedback\nfor v2:\n\n* suppressable advice on restoring if a user deletes a branch\nvia @{-x} or - reference (to ensure safety: if a user deleted\nthe wrong branch instead what she thought is @{-1}, which seems\nto be more likely compared with the situation when branch name\nhas to be typed in)\n\n* if not enough switches exist to delete branch via @{-x} or -\nreference, a corresponding warning is displayed \n\nThank you! Looking forward to any feedback.\n\n Documentation/git-branch.txt | 2 ++\n advice.c | 10 ++++++++++\n advice.h | 2 ++\n builtin/branch.c | 22 +++++++++++++++++++---\n t/t3200-branch.sh | 10 ++++++++++\n 5 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 4a7037f..42b96ed 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -65,6 +65,8 @@ to happen.\n With a `-d` or `-D` option, `` will be deleted. You may\n specify more than one branch for deletion. If the branch currently\n has a reflog then the reflog will also be deleted.\n+The \"@{-N}\" syntax for the N-th last branch deletes the specified branch.\n+You may also specify - which is synonymous with \"@{-1}\".\n \n Use `-r` together with `-d` to delete remote-tracking branches. Note, that it\n only makes sense to delete remote-tracking branches if they no longer exist\ndiff --git a/advice.c b/advice.c\nindex 4dc5cf1..f14eb68 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -15,6 +15,7 @@ int advice_detached_head = 1;\n int advice_set_upstream_failure = 1;\n int advice_object_name_warning = 1;\n int advice_rm_hints = 1;\n+int advice_delete_branch_via_at_ref = 1;\n \n static struct {\n \tconst char *name;\n@@ -35,6 +36,7 @@ static struct {\n \t{ \"setupstreamfailure\", &advice_set_upstream_failure },\n \t{ \"objectnamewarning\", &advice_object_name_warning },\n \t{ \"rmhints\", &advice_rm_hints },\n+\t{ \"deletebranchviaatref\", &advice_delete_branch_via_at_ref },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushnonfastforward\", &advice_push_update_rejected }\n@@ -117,3 +119,11 @@ void detach_advice(const char *new_name)\n \n \tfprintf(stderr, fmt, new_name);\n }\n+\n+void delete_branch_advice(const char *name, const char *ref)\n+{\n+\tconst char fmt[] =\n+\t\"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n+\n+\tfprintf(stderr, fmt, name, ref);\n+}\ndiff --git a/advice.h b/advice.h\nindex b341a55..192eef7 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -18,6 +18,7 @@ extern int advice_detached_head;\n extern int advice_set_upstream_failure;\n extern int advice_object_name_warning;\n extern int advice_rm_hints;\n+extern int advice_delete_branch_via_at_ref;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\n@@ -26,5 +27,6 @@ int error_resolve_conflict(const char *me);\n extern void NORETURN die_resolve_conflict(const char *me);\n void NORETURN die_conclude_merge(void);\n void detach_advice(const char *new_name);\n+void delete_branch_advice(const char *name, const char *ref);\n \n #endif /* ADVICE_H */\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 7b45b6b..4f5ec72 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -178,6 +178,12 @@ static void delete_branch_config(const char *branchname)\n \tstrbuf_release(&buf);\n }\n \n+static void expand_dash_shortcut(const char **argv, int dash_position)\n+{\n+\tif (!strcmp(argv[dash_position], \"-\"))\n+\t\targv[dash_position] = \"@{-1}\";\n+}\n+\n static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\t int quiet)\n {\n@@ -187,6 +193,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \tconst char *fmt;\n \tint i;\n \tint ret = 0;\n+\tint at_shortcut = 0;\n \tint remote_branch = 0;\n \tstruct strbuf bname = STRBUF_INIT;\n \n@@ -214,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tconst char *target;\n \t\tint flags = 0;\n \n+\t\texpand_dash_shortcut (argv, i);\n+\t\tif(!strncmp(argv[i], \"@{-\", strlen(\"@{-\")))\n+\t\t\tat_shortcut = 1;\n \t\tstrbuf_branchname(&bname, argv[i]);\n \t\tif (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {\n \t\t\terror(_(\"Cannot delete the branch '%s' \"\n@@ -231,9 +241,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\t\t\t | RESOLVE_REF_ALLOW_BAD_NAME,\n \t\t\t\t\t sha1, &flags);\n \t\tif (!target) {\n-\t\t\terror(remote_branch\n-\t\t\t ? _(\"remote-tracking branch '%s' not found.\")\n-\t\t\t : _(\"branch '%s' not found.\"), bname.buf);\n+\t\t\terror((!strncmp(bname.buf, \"@{-\", strlen(\"@{-\")))\n+\t\t\t\t? _(\"There is not enough branch switches to\"\n+\t\t\t\t\t\" delete '%s'.\")\n+\t\t\t\t: remote_branch\n+\t\t\t\t\t? _(\"remote-tracking branch '%s' not found.\")\n+\t\t\t\t\t: _(\"branch '%s' not found.\"), bname.buf);\n \t\t\tret = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -262,6 +275,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\t (flags & REF_ISBROKEN) ? \"broken\"\n \t\t\t : (flags & REF_ISSYMREF) ? target\n \t\t\t : find_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\t\t\tif (at_shortcut && advice_delete_branch_via_at_ref)\n+\t\t\t delete_branch_advice (bname.buf,\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \t\t}\n \t\tdelete_branch_config(bname.buf);\n \t}\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex a897248..0b59c94 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -372,6 +372,16 @@ test_expect_success 'test overriding tracking setup via --no-track' '\n \t! test \"$(git config branch.my2.merge)\" = refs/heads/master\n '\n \n+test_expect_success 'test deleting \"-\" deletes previous branch' '\n+\tgit checkout -b prev &&\n+\ttest_commit prev &&\n+\tgit checkout master &&\n+\tgit branch -D - >actual &&\n+\tsha1=$(git rev-parse prev | cut -c 1-7) &&\n+\techo \"Deleted branch prev (was $sha1).\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'no tracking without .fetch entries' '\n \tgit config branch.autosetupmerge true &&\n \tgit branch my6 s &&\n-- \n2.8.0.dirty\n"},{"id":"282325","messageId":"vpqwpoi1y72.fsf@anie.imag.fr","threadId":"41882","inReplyTo":"1459416327-795-1-git-send-email-elena.petrashen@gmail.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-03-31T15:09:21Z","receivedAt":"2016-03-31T15:09:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Elena Petrashen writes:\n\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -15,6 +15,7 @@ int advice_detached_head = 1;\n> int advice_set_upstream_failure = 1;\n> int advice_object_name_warning = 1;\n> int advice_rm_hints = 1;\n> +int advice_delete_branch_via_at_ref = 1;\n> \n> static struct {\n> \tconst char *name;\n> @@ -35,6 +36,7 @@ static struct {\n> \t{ \"setupstreamfailure\", &advice_set_upstream_failure },\n> \t{ \"objectnamewarning\", &advice_object_name_warning },\n> \t{ \"rmhints\", &advice_rm_hints },\n> +\t{ \"deletebranchviaatref\", &advice_delete_branch_via_at_ref },\n> \n> \t/* make this an alias for backward compatibility */\n> \t{ \"pushnonfastforward\", &advice_push_update_rejected }\n\nThis is undocumented.\n\n> +\t\texpand_dash_shortcut (argv, i);\n> +\t\tif(!strncmp(argv[i], \"@{-\", strlen(\"@{-\")))\n\nStyle: space after if.\n\nWe have a starts_with() function to avoid this strncmp(... strlen()) dance.\n\n> @@ -262,6 +275,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n> \t\t\t (flags & REF_ISBROKEN) ? \"broken\"\n> \t\t\t : (flags & REF_ISSYMREF) ? target\n> \t\t\t : find_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\t\t\tif (at_shortcut && advice_delete_branch_via_at_ref)\n> +\t\t\t delete_branch_advice (bname.buf,\n> +\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n\nStyle: 1-space indent is not sufficient. Either align the arguments (not\napplicable here) or indent by one tab to mark \"I'm not done with\nargument list\".\n\nAlso, no space between delete_branch_advice and '('.\n\n> +test_expect_success 'test deleting \"-\" deletes previous branch' '\n> +\tgit checkout -b prev &&\n> +\ttest_commit prev &&\n> +\tgit checkout master &&\n> +\tgit branch -D - >actual &&\n> +\tsha1=$(git rev-parse prev | cut -c 1-7) &&\n\nDidn't this | cut ... turn into a --short in a previous version?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"282326","messageId":"972993506.2587877.1459438308676.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"41882","inReplyTo":"1459416327-795-1-git-send-email-elena.petrashen@gmail.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-03-31T15:31:48Z","receivedAt":"2016-03-31T15:31:48Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Elena Petrashen wrote:\n> +void delete_branch_advice(const char *name, const char *ref)\n> +{\n> + const char fmt[] =\n> + \"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n\nShouldn't that be marked for translation, like is done with the other\nstrings?\n\nThanks,\nRémi\n"},{"id":"282375","messageId":"xmqqmvpemot7.fsf@gitster.mtv.corp.google.com","threadId":"41882","inReplyTo":"1459416327-795-1-git-send-email-elena.petrashen@gmail.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T19:26:28Z","receivedAt":"2016-03-31T19:26:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elena Petrashen writes:\n\n> @@ -214,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n> \t\tconst char *target;\n> \t\tint flags = 0;\n> \n> +\t\texpand_dash_shortcut (argv, i);\n> +\t\tif(!strncmp(argv[i], \"@{-\", strlen(\"@{-\")))\n> +\t\t\tat_shortcut = 1;\n> \t\tstrbuf_branchname(&bname, argv[i]);\n> \t\tif (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {\n> \t\t\terror(_(\"Cannot delete the branch '%s' \"\n> @@ -231,9 +241,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n> \t\t\t\t\t | RESOLVE_REF_ALLOW_BAD_NAME,\n> \t\t\t\t\t sha1, &flags);\n> \t\tif (!target) {\n> -\t\t\terror(remote_branch\n> -\t\t\t ? _(\"remote-tracking branch '%s' not found.\")\n> -\t\t\t : _(\"branch '%s' not found.\"), bname.buf);\n> +\t\t\terror((!strncmp(bname.buf, \"@{-\", strlen(\"@{-\")))\n> +\t\t\t\t? _(\"There is not enough branch switches to\"\n> +\t\t\t\t\t\" delete '%s'.\")\n> +\t\t\t\t: remote_branch\n> +\t\t\t\t\t? _(\"remote-tracking branch '%s' not found.\")\n> +\t\t\t\t\t: _(\"branch '%s' not found.\"), bname.buf);\n\nI was expecting that the check for \"@{-\" in bname.buf would be done\nimmediately after strbuf_branchname(&bname, argv[i]) we see in the\nprevious hunk (and an error message issued there), i.e. something\nlike:\n\n orig_arg = argv[i];\n if (!strcmp(orig_arg, \"-\"))\n\t\tstrbuf_branchname(&bname, \"@{-1}\");\n\telse\n\t\tstrbuf_branchname(&bname, argv[i]);\n if (starts_with(bname.buf, \"@{-\")) {\n\t\terror(\"Not enough branch switches to delete %s\", orig_arg);\n ... clean up and fail ...\n\t}\n\nThat would give you sensible error message for \"branch -d -\",\n\"branch -d @{-1}\" and \"branch -d @{-4}\" if you haven't visited\ndifferent branches enough times.\n\nThe hope was that the remainder of the code (including this error\nmessage) would not have to worry about this \"not enough switches\"\nerror at all if done that way.\n\n> @@ -262,6 +275,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n> \t\t\t (flags & REF_ISBROKEN) ? \"broken\"\n> \t\t\t : (flags & REF_ISSYMREF) ? target\n> \t\t\t : find_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\t\t\tif (at_shortcut && advice_delete_branch_via_at_ref)\n> +\t\t\t delete_branch_advice (bname.buf,\n> +\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n> \t\t}\n\nThe existing !quiet report already said \"deleted branch\" with the\nconcrete branch name, not \"@{-1}\" or \"-\", taken from bname.buf at\nthis point.\n\nIf the advice on how to recover a deletion by mistake would help the\nuser, wouldn't that apply equally to the case where the user made a\ntypo in the original command line, i.e. \"branch -d foo\" when she\nmeant to delete \"branch -d fooo\", as well? If we drop the \"at_shortcut\"\ncheck from this if() statement, wouldn't the result be more helpful?\n\nThanks\n"},{"id":"282645","messageId":"CAJPOeMeqMpy8wmZ3jMfw_XLAS2WbPOtJF8_Rd_sPGQNeCNyFnw@mail.gmail.com","threadId":"41882","inReplyTo":"972993506.2587877.1459438308676.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"elena petrashen","fromEmail":"elena.petrashen@gmail.com","sentAt":"2016-04-04T20:31:05Z","receivedAt":"2016-04-04T20:31:05Z","isPatch":true,"sender":{"key":"elena.petrashen@gmail.com","avatar":"https://gravatar.com/avatar/a2fd27bdf84eb2b7fc7b034a358aa6777fe86d5a9ac9339f122aea2b6b7689d4?d=mp&s=160"},"body":"On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso\n wrote:\n> Elena Petrashen wrote:\n>> +void delete_branch_advice(const char *name, const char *ref)\n>> +{\n>> + const char fmt[] =\n>> + \"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n>\n> Shouldn't that be marked for translation, like is done with the other\n> strings?\n>\n> Thanks,\n> Rémi\n\nThank you for letting me know about that! Could you please\nhelp me out and explain how do I mark it for translation? I tried\nto do it the same way as with the other strings but evidently\ndidn't quite succeed.\n\nThanks!\nElena\n"},{"id":"282651","messageId":"1048705176.2897167.1459806794368.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"41882","inReplyTo":"CAJPOeMeqMpy8wmZ3jMfw_XLAS2WbPOtJF8_Rd_sPGQNeCNyFnw@mail.gmail.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-04-04T21:53:14Z","receivedAt":"2016-04-04T21:53:14Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"elena petrashen wrote:\n> On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso\n> wrote:\n> > Elena Petrashen wrote:\n> >> +void delete_branch_advice(const char *name, const char *ref)\n> >> +{\n> >> + const char fmt[] =\n> >> + \"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n> >\n> > Shouldn't that be marked for translation, like is done with the other\n> > strings?\n> >\n> > Thanks,\n> > Rémi\n> \n> Thank you for letting me know about that! Could you please\n> help me out and explain how do I mark it for translation? I tried\n> to do it the same way as with the other strings but evidently\n> didn't quite succeed.\n\nI am not sure.\nI tried to grep similar cases, it seems that you can do the following:\n\n\tconst char fmt[] = N_(\"\\nNote: to restore [...] \\ngit branch %s %s\\n\");\n\tfprintf(stderr, _(fmt), name, ref);\n\nSome similar example in builtin/add.c:\n\n\tstatic const char ignore_error[] =\n\tN_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n\t[...]\n\t\t\tfprintf(stderr, _(ignore_error));\n\nOr you can define fmt as a 'const char *' and in that case do the\nfollowing:\n\n\tconst char *fmt = _(\"\\nNote: to restore [...] \\n git branch %s %s\\n\");\n\tfprintf(stderr, fmt, name, ref);\n\n\nIn builtin/am.c:\n\t\tconst char *invalid_line = _(\"Malformed input line: '%s'.\");\n\t[...]\n\t\t\t\tret = error(invalid_line, sb.buf);\n\nI don't know which one is the best way to go though.\n\nThanks,\nRémi\n"},{"id":"282758","messageId":"CAJPOeMeAQVR9ZvEC+r24P-RZq46iAeVx3CrLNbGXBGE8jw-iUA@mail.gmail.com","threadId":"41882","inReplyTo":"1048705176.2897167.1459806794368.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"elena petrashen","fromEmail":"elena.petrashen@gmail.com","sentAt":"2016-04-06T10:00:25Z","receivedAt":"2016-04-06T10:00:25Z","isPatch":true,"sender":{"key":"elena.petrashen@gmail.com","avatar":"https://gravatar.com/avatar/a2fd27bdf84eb2b7fc7b034a358aa6777fe86d5a9ac9339f122aea2b6b7689d4?d=mp&s=160"},"body":"On Tue, Apr 5, 2016 at 12:53 AM, Remi Galan Alfonso\n wrote:\n> elena petrashen wrote:\n>> On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso\n>> wrote:\n>> > Elena Petrashen wrote:\n>> >> +void delete_branch_advice(const char *name, const char *ref)\n>> >> +{\n>> >> + const char fmt[] =\n>> >> + \"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n>> >\n>> > Shouldn't that be marked for translation, like is done with the other\n>> > strings?\n>> >\n>> > Thanks,\n>> > Rémi\n>>\n>> Thank you for letting me know about that! Could you please\n>> help me out and explain how do I mark it for translation? I tried\n>> to do it the same way as with the other strings but evidently\n>> didn't quite succeed.\n>\n> I am not sure.\n> I tried to grep similar cases, it seems that you can do the following:\n>\n> const char fmt[] = N_(\"\\nNote: to restore [...] \\ngit branch %s %s\\n\");\n> fprintf(stderr, _(fmt), name, ref);\n>\n> Some similar example in builtin/add.c:\n>\n> static const char ignore_error[] =\n> N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n> [...]\n> fprintf(stderr, _(ignore_error));\n>\n> Or you can define fmt as a 'const char *' and in that case do the\n> following:\n>\n> const char *fmt = _(\"\\nNote: to restore [...] \\n git branch %s %s\\n\");\n> fprintf(stderr, fmt, name, ref);\n>\n>\n> In builtin/am.c:\n> const char *invalid_line = _(\"Malformed input line: '%s'.\");\n> [...]\n> ret = error(invalid_line, sb.buf);\n>\n> I don't know which one is the best way to go though.\n>\n> Thanks,\n> Rémi\n\nThank you! I'm a bit confused though as the previous implemented advice function\nin advice.c (the one I used for reference) doesn't seem to employ this... does\nthis mean it's also not marked for translation?\n\nvoid detach_advice(const char *new_name)\n{\nconst char fmt[] =\n\"Note: checking out '%s'.\\n\\n\"\n\"You are in 'detached HEAD' state. <.........>Example:\\n\\n\"\n\" git checkout -b \\n\\n\";\n\nfprintf(stderr, fmt, new_name);\n}\n\nCheers,\nElena\n"},{"id":"282760","messageId":"CAJPOeMd9hJpVubRbSTsBGBHtFSi5-LsQOr=h=Gtu+7qZgq_Fbg@mail.gmail.com","threadId":"41882","inReplyTo":"xmqqmvpemot7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"elena petrashen","fromEmail":"elena.petrashen@gmail.com","sentAt":"2016-04-06T10:42:29Z","receivedAt":"2016-04-06T10:42:29Z","isPatch":true,"sender":{"key":"elena.petrashen@gmail.com","avatar":"https://gravatar.com/avatar/a2fd27bdf84eb2b7fc7b034a358aa6777fe86d5a9ac9339f122aea2b6b7689d4?d=mp&s=160"},"body":"On Thu, Mar 31, 2016 at 10:26 PM, Junio C Hamano wrote:\n> Elena Petrashen writes:\n>\n>> @@ -214,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n>> const char *target;\n>> int flags = 0;\n>>\n>> + expand_dash_shortcut (argv, i);\n>> + if(!strncmp(argv[i], \"@{-\", strlen(\"@{-\")))\n>> + at_shortcut = 1;\n>> strbuf_branchname(&bname, argv[i]);\n>> if (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {\n>> error(_(\"Cannot delete the branch '%s' \"\n>> @@ -231,9 +241,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n>> | RESOLVE_REF_ALLOW_BAD_NAME,\n>> sha1, &flags);\n>> if (!target) {\n>> - error(remote_branch\n>> - ? _(\"remote-tracking branch '%s' not found.\")\n>> - : _(\"branch '%s' not found.\"), bname.buf);\n>> + error((!strncmp(bname.buf, \"@{-\", strlen(\"@{-\")))\n>> + ? _(\"There is not enough branch switches to\"\n>> + \" delete '%s'.\")\n>> + : remote_branch\n>> + ? _(\"remote-tracking branch '%s' not found.\")\n>> + : _(\"branch '%s' not found.\"), bname.buf);\n>\n> I was expecting that the check for \"@{-\" in bname.buf would be done\n> immediately after strbuf_branchname(&bname, argv[i]) we see in the\n> previous hunk (and an error message issued there), i.e. something\n> like:\n>\n> orig_arg = argv[i];\n> if (!strcmp(orig_arg, \"-\"))\n> strbuf_branchname(&bname, \"@{-1}\");\n> else\n> strbuf_branchname(&bname, argv[i]);\n> if (starts_with(bname.buf, \"@{-\")) {\n> error(\"Not enough branch switches to delete %s\", orig_arg);\n> ... clean up and fail ...\n> }\n>\n> That would give you sensible error message for \"branch -d -\",\n> \"branch -d @{-1}\" and \"branch -d @{-4}\" if you haven't visited\n> different branches enough times.\n>\n> The hope was that the remainder of the code (including this error\n> message) would not have to worry about this \"not enough switches\"\n> error at all if done that way.\n\nThank you, and I apologize it takes me kind of long to figure it all right.\nFor me the reason I did the realisation the way it is it's to distinguish the\nerror messages in cases:\n1. the branch @{-1} was deleted already\n2. the \"not enough switches case\", there was no previous branch\n\nas far as I understand in #1 we should recieve \"branch foo was not found\"\nin #2 \"not enough swiches to delete @{-1}\". I believe if we check for \"@{-\"\nimmediately, there would be no opportunity to distinguish, and we will be\ngetting \"not enough swithes\" even if there was enough switches, it's just\nthat the branch was deleted?\n\n>\n>> @@ -262,6 +275,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n>> (flags & REF_ISBROKEN) ? \"broken\"\n>> : (flags & REF_ISSYMREF) ? target\n>> : find_unique_abbrev(sha1, DEFAULT_ABBREV));\n>> + if (at_shortcut && advice_delete_branch_via_at_ref)\n>> + delete_branch_advice (bname.buf,\n>> + find_unique_abbrev(sha1, DEFAULT_ABBREV));\n>> }\n>\n> The existing !quiet report already said \"deleted branch\" with the\n> concrete branch name, not \"@{-1}\" or \"-\", taken from bname.buf at\n> this point.\n>\n> If the advice on how to recover a deletion by mistake would help the\n> user, wouldn't that apply equally to the case where the user made a\n> typo in the original command line, i.e. \"branch -d foo\" when she\n> meant to delete \"branch -d fooo\", as well? If we drop the \"at_shortcut\"\n> check from this if() statement, wouldn't the result be more helpful?\n>\n> Thanks\n\nWouldn't people most of the time have somewhat more different names\nthan foo/fooo/foooo? Anyways, I guess that should be reasonable. Will just\nadd advice for every branch deletion next time.\n\nThank you!\n"},{"id":"282788","messageId":"980248197.3116219.1459973119988.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"41882","inReplyTo":"CAJPOeMeAQVR9ZvEC+r24P-RZq46iAeVx3CrLNbGXBGE8jw-iUA@mail.gmail.com","subject":"Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-04-06T20:05:19Z","receivedAt":"2016-04-06T20:05:19Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"elena petrashen wrote:\n> On Tue, Apr 5, 2016 at 12:53 AM, Remi Galan Alfonso\n> wrote:\n> > elena petrashen wrote:\n> >> On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso\n> >> wrote:\n> >> > Elena Petrashen wrote:\n> >> >> +void delete_branch_advice(const char *name, const char *ref)\n> >> >> +{\n> >> >> + const char fmt[] =\n> >> >> + \"\\nNote: to restore the deleted branch:\\n\\ngit branch %s %s\\n\";\n> >> >\n> >> > Shouldn't that be marked for translation, like is done with the other\n> >> > strings?\n> >> >\n> >> > Thanks,\n> >> > Rémi\n> >>\n> >> Thank you for letting me know about that! Could you please\n> >> help me out and explain how do I mark it for translation? I tried\n> >> to do it the same way as with the other strings but evidently\n> >> didn't quite succeed.\n> >\n> > I am not sure.\n> > I tried to grep similar cases, it seems that you can do the following:\n> >\n> > const char fmt[] = N_(\"\\nNote: to restore [...] \\ngit branch %s %s\\n\");\n> > fprintf(stderr, _(fmt), name, ref);\n> >\n> > Some similar example in builtin/add.c:\n> >\n> > static const char ignore_error[] =\n> > N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n> > [...]\n> > fprintf(stderr, _(ignore_error));\n> >\n> > Or you can define fmt as a 'const char *' and in that case do the\n> > following:\n> >\n> > const char *fmt = _(\"\\nNote: to restore [...] \\n git branch %s %s\\n\");\n> > fprintf(stderr, fmt, name, ref);\n> >\n> >\n> > In builtin/am.c:\n> > const char *invalid_line = _(\"Malformed input line: '%s'.\");\n> > [...]\n> > ret = error(invalid_line, sb.buf);\n> >\n> > I don't know which one is the best way to go though.\n> >\n> > Thanks,\n> > Rémi\n> \n> Thank you! I'm a bit confused though as the previous implemented advice function\n> in advice.c (the one I used for reference) doesn't seem to employ this... does\n> this mean it's also not marked for translation?\n> \n> void detach_advice(const char *new_name)\n> {\n> const char fmt[] =\n> \"Note: checking out '%s'.\\n\\n\"\n> \"You are in 'detached HEAD' state. <.........>Example:\\n\\n\"\n> \" git checkout -b \\n\\n\";\n> \n> fprintf(stderr, fmt, new_name);\n> }\n\nIt doesn't seem so, at least it doesn't translate:\n\n# $ LANG=fr_FR.UTF-8 git checkout 9494c39\n# Note: checking out '9494c39'.\n# \n# You are in 'detached HEAD' state. You can look around, make experimental\n# changes and commit them, and you can discard any commits you make in this\n# state without impacting any branches by performing another checkout.\n# \n# If you want to create a new branch to retain commits you create, you may\n# do so (now or later) by using -b with the checkout command again. Example:\n# \n# git checkout -b \n# \n# HEAD est maintenant sur 9494c39... Sync with Git 2.8.1\nOnly the last line is translated.\n\nBlame traces it back to commit 2857093b that was in 2012 so obviously\nthe lack of translation isn't due to the message being recent.\n\nThanks,\nRémi\n"}]}