{"thread":{"id":"41240","subject":"[PATCH] status: be prepared for not-yet-started interactive rebase","startedAt":"2016-01-22T16:28:40Z","lastAt":"2017-01-27T10:52:18Z","messageCount":10,"participants":["Johannes Schindelin","Matthieu Moy","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"276542","messageId":"99f6de4be107044fdf01ee796f42e124ac147891.1453480067.git.johannes.schindelin@gmx.de","threadId":"41240","inReplyTo":null,"subject":"[PATCH] status: be prepared for not-yet-started interactive rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-22T16:28:40Z","receivedAt":"2016-01-22T16:28:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Junio C Hamano \n\nSome developers might want to call `git status` in a working\ndirectory where they just started an interactive rebase, but the\nedit script is still opened in the editor.\n\nLet's show a meaningful message in such cases.\n\n[jes: suppressed the \"No commands remaining\" message when the\ninteractive rebase has not started yet.]\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n wt-status.c | 22 +++++++++++++++-------\n 1 file changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex bba2596..ce8dfda 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1068,14 +1068,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)\n \n }\n \n-static void read_rebase_todolist(const char *fname, struct string_list *lines)\n+static int read_rebase_todolist(const char *fname, struct string_list *lines)\n {\n \tstruct strbuf line = STRBUF_INIT;\n \tFILE *f = fopen(git_path(\"%s\", fname), \"r\");\n \n-\tif (!f)\n+\tif (!f) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn -1;\n \t\tdie_errno(\"Could not open file %s for reading\",\n \t\t\t git_path(\"%s\", fname));\n+\t}\n \twhile (!strbuf_getline(&line, f, '\\n')) {\n \t\tif (line.len && line.buf[0] == comment_line_char)\n \t\t\tcontinue;\n@@ -1085,6 +1088,7 @@ static void read_rebase_todolist(const char *fname, struct string_list *lines)\n \t\tabbrev_sha1_in_line(&line);\n \t\tstring_list_append(lines, line.buf);\n \t}\n+\treturn 0;\n }\n \n static void show_rebase_information(struct wt_status *s,\n@@ -1098,10 +1102,12 @@ static void show_rebase_information(struct wt_status *s,\n \t\tstruct string_list have_done = STRING_LIST_INIT_DUP;\n \t\tstruct string_list yet_to_do = STRING_LIST_INIT_DUP;\n \n-\t\tread_rebase_todolist(\"rebase-merge/done\", &have_done);\n-\t\tread_rebase_todolist(\"rebase-merge/git-rebase-todo\", &yet_to_do);\n-\n-\t\tif (have_done.nr == 0)\n+\t\tif ((read_rebase_todolist(\"rebase-merge/done\", &have_done)) ||\n+\t\t (read_rebase_todolist(\"rebase-merge/git-rebase-todo\",\n+\t\t\t\t &yet_to_do)))\n+\t\t\tstatus_printf_ln(s, color,\n+\t\t\t\t_(\"rebase-i not started yet.\"));\n+\t\telse if (have_done.nr == 0)\n \t\t\tstatus_printf_ln(s, color, _(\"No commands done.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n@@ -1119,7 +1125,9 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\t\t\t_(\" (see more in file %s)\"), git_path(\"rebase-merge/done\"));\n \t\t}\n \n-\t\tif (yet_to_do.nr == 0)\n+\t\tif (have_done.nr == 0)\n+\t\t\t; /* do nothing */\n+\t\telse if (yet_to_do.nr == 0)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"No commands remaining.\"));\n \t\telse {\n-- \n2.7.0.windows.1.7.g55a05c8\n"},{"id":"276543","messageId":"vpqlh7h5zrh.fsf@anie.imag.fr","threadId":"41240","inReplyTo":"99f6de4be107044fdf01ee796f42e124ac147891.1453480067.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] status: be prepared for not-yet-started interactive rebase","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-01-22T16:45:38Z","receivedAt":"2016-01-22T16:45:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin writes:\n\n> wt-status.c | 22 +++++++++++++++-------\n> 1 file changed, 15 insertions(+), 7 deletions(-)\n\nLooks good to me. You may want to add a test by overriding $EDITOR to a\nscript doing \"git status >actual\" if you want to have fun with testing.\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"276550","messageId":"alpine.DEB.2.20.1601221811030.2964@virtualbox","threadId":"41240","inReplyTo":"vpqlh7h5zrh.fsf@anie.imag.fr","subject":"Re: [PATCH] status: be prepared for not-yet-started interactive rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-22T17:11:48Z","receivedAt":"2016-01-22T17:11:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matthieu,\n\nOn Fri, 22 Jan 2016, Matthieu Moy wrote:\n\n> Johannes Schindelin writes:\n> \n> > wt-status.c | 22 +++++++++++++++-------\n> > 1 file changed, 15 insertions(+), 7 deletions(-)\n> \n> Looks good to me. You may want to add a test by overriding $EDITOR to a\n> script doing \"git status >actual\" if you want to have fun with testing.\n\nSadly, I lack the time.\n\nCiao,\nDscho\n"},{"id":"276560","messageId":"xmqqio2lv4rg.fsf@gitster.mtv.corp.google.com","threadId":"41240","inReplyTo":"vpqlh7h5zrh.fsf@anie.imag.fr","subject":"Re: [PATCH] status: be prepared for not-yet-started interactive rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-22T18:38:27Z","receivedAt":"2016-01-22T18:38:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Johannes Schindelin writes:\n>\n>> wt-status.c | 22 +++++++++++++++-------\n>> 1 file changed, 15 insertions(+), 7 deletions(-)\n>\n> Looks good to me. You may want to add a test by overriding $EDITOR to a\n> script doing \"git status >actual\" if you want to have fun with testing.\n\nI am unhappy that the code does not read 'rebase-todo' at all when\n'done' is missing.\n\nIf we cannot read 'todo', that would mean we shouldn't be in this\nfunction in the first place, which is a sign of something more\nserious; somebody created 'interactive' but did not leave 'todo' to\nread for us--why?\n\nA missing 'done' is much more benign and making us not to barf is a\nworthy thing to do, but we shouldn't be removing protection against\na more serious error as its side effect. That is what I am unhappy\nabout this change.\n"},{"id":"276565","messageId":"alpine.DEB.2.20.1601222000040.2964@virtualbox","threadId":"41240","inReplyTo":"xmqqio2lv4rg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] status: be prepared for not-yet-started interactive rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-01-22T19:02:45Z","receivedAt":"2016-01-22T19:02:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 22 Jan 2016, Junio C Hamano wrote:\n\n> Matthieu Moy writes:\n> \n> > Johannes Schindelin writes:\n> >\n> >> wt-status.c | 22 +++++++++++++++-------\n> >> 1 file changed, 15 insertions(+), 7 deletions(-)\n> >\n> > Looks good to me. You may want to add a test by overriding $EDITOR to a\n> > script doing \"git status >actual\" if you want to have fun with testing.\n> \n> I am unhappy that the code does not read 'rebase-todo' at all when\n> 'done' is missing.\n\nAt this point is virtually certain that the edit script is opened in an\neditor, and very likely to change. So it makes little sense to look at the\ntodo.\n\nCiao,\nDscho\n"},{"id":"310330","messageId":"cover.1485446899.git.johannes.schindelin@gmx.de","threadId":"41240","inReplyTo":"99f6de4be107044fdf01ee796f42e124ac147891.1453480067.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/1] Let `git status` handle a not-yet-started `rebase -i` gracefully","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-26T16:08:29Z","receivedAt":"2017-01-26T16:16:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When the `done` file is missing, we die()d. This is not necessary, we\ncan do much better than that.\n\nChanges since v1:\n\n- When `done` is missing, we still read `git-rebase-todo` and report the\n next steps.\n\n- We now report a missing git-rebase-todo.\n\n- Added a test (thanks, Matthieu, for prodding me into working harder\n ;-)).\n\n- As I changed so much, I took authorship of the patch.\n\n\nJohannes Schindelin (1):\n status: be prepared for not-yet-started interactive rebase\n\n t/t7512-status-help.sh | 19 +++++++++++++++++++\n wt-status.c | 14 ++++++++++----\n 2 files changed, 29 insertions(+), 4 deletions(-)\n\n\nbase-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe\nPublished-As: https://github.com/dscho/git/releases/tag/wt-status-v2\nFetch-It-Via: git fetch https://github.com/dscho/git wt-status-v2\n\nInterdiff vs v1:\n\n diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\n index 5c3db656df..458608cc1e 100755\n --- a/t/t7512-status-help.sh\n +++ b/t/t7512-status-help.sh\n @@ -944,4 +944,23 @@ EOF\n \ttest_i18ncmp expected actual\n '\n \n +test_expect_success 'status: handle not-yet-started rebase -i gracefully' '\n +\tONTO=$(git rev-parse --short HEAD^) &&\n +\tCOMMIT=$(git rev-parse --short HEAD) &&\n +\tEDITOR=\"git status --untracked-files=no >actual\" git rebase -i HEAD^ &&\n +\tcat >expected <\n---\n t/t7512-status-help.sh | 19 +++++++++++++++++++\n wt-status.c | 14 ++++++++++----\n 2 files changed, 29 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex 5c3db656df..458608cc1e 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -944,4 +944,23 @@ EOF\n \ttest_i18ncmp expected actual\n '\n \n+test_expect_success 'status: handle not-yet-started rebase -i gracefully' '\n+\tONTO=$(git rev-parse --short HEAD^) &&\n+\tCOMMIT=$(git rev-parse --short HEAD) &&\n+\tEDITOR=\"git status --untracked-files=no >actual\" git rebase -i HEAD^ &&\n+\tcat >expected < writes:\n\n> Some developers might want to call `git status` in a working\n> directory where they just started an interactive rebase, but the\n> edit script is still opened in the editor.\n>\n> Let's show a meaningful message in such cases.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t7512-status-help.sh | 19 +++++++++++++++++++\n> wt-status.c | 14 ++++++++++----\n> 2 files changed, 29 insertions(+), 4 deletions(-)\n\nThe patch looks good to me.\n\n> @@ -1166,8 +1170,10 @@ static void show_rebase_information(struct wt_status *s,\n> \t\tstruct string_list yet_to_do = STRING_LIST_INIT_DUP;\n> \n> \t\tread_rebase_todolist(\"rebase-merge/done\", &have_done);\n> -\t\tread_rebase_todolist(\"rebase-merge/git-rebase-todo\", &yet_to_do);\n> -\n> +\t\tif (read_rebase_todolist(\"rebase-merge/git-rebase-todo\",\n> +\t\t\t\t\t &yet_to_do))\n> +\t\t\tstatus_printf_ln(s, color,\n> +\t\t\t\t_(\"git-rebase-todo is missing.\"));\n\nI first was surprised not to see this \"git-rebase-todo\" in the output of\nstatus, but the testcase tests a missing 'done', not a missing todo, so\nit's normal.\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"310337","messageId":"CAGZ79kYLFJYPQu5KSv3hG+_eavO9BHkxHjpVOEs63Nn6Hu1gTg@mail.gmail.com","threadId":"41240","inReplyTo":"alpine.DEB.2.20.1701261708370.3469@virtualbox","subject":"Re: [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-26T18:02:02Z","receivedAt":"2017-01-26T18:12:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 26, 2017 at 8:08 AM, Johannes Schindelin\n wrote:\n> Some developers might want to call `git status` in a working\n> directory where they just started an interactive rebase, but the\n> edit script is still opened in the editor.\n>\n> Let's show a meaningful message in such cases.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t7512-status-help.sh | 19 +++++++++++++++++++\n> wt-status.c | 14 ++++++++++----\n> 2 files changed, 29 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\n> index 5c3db656df..458608cc1e 100755\n> --- a/t/t7512-status-help.sh\n> +++ b/t/t7512-status-help.sh\n> @@ -944,4 +944,23 @@ EOF\n> test_i18ncmp expected actual\n> '\n>\n> +test_expect_success 'status: handle not-yet-started rebase -i gracefully' '\n> + ONTO=$(git rev-parse --short HEAD^) &&\n> + COMMIT=$(git rev-parse --short HEAD) &&\n> + EDITOR=\"git status --untracked-files=no >actual\" git rebase -i HEAD^ &&\n> + cat >expected < +On branch several_commits\n> +No commands done.\n> +Next command to do (1 remaining command):\n> + pick $COMMIT four_commit\n> + (use \"git rebase --edit-todo\" to view and edit)\n> +You are currently editing a commit while rebasing branch '\\''several_commits'\\'' on '\\''$ONTO'\\''.\n> + (use \"git commit --amend\" to amend the current commit)\n> + (use \"git rebase --continue\" once you are satisfied with your changes)\n> +\n> +nothing to commit (use -u to show untracked files)\n> +EOF\n> + test_i18ncmp expected actual\n> +'\n> +\n> test_done\n> diff --git a/wt-status.c b/wt-status.c\n> index a715e71906..4dff0b3e21 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1135,14 +1135,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)\n> strbuf_list_free(split);\n> }\n>\n> -static void read_rebase_todolist(const char *fname, struct string_list *lines)\n> +static int read_rebase_todolist(const char *fname, struct string_list *lines)\n> {\n> struct strbuf line = STRBUF_INIT;\n> FILE *f = fopen(git_path(\"%s\", fname), \"r\");\n>\n> - if (!f)\n> + if (!f) {\n> + if (errno == ENOENT)\n> + return -1;\n> die_errno(\"Could not open file %s for reading\",\n> git_path(\"%s\", fname));\n\nWhile at it, fix the translation with die_errno(_(..),..) ?\n(The errno message is translated already by the system,\nwhich make untranslated die_errno things awkward for the users.)\n\nOtherwise the patch looks good to me\n"},{"id":"310402","messageId":"alpine.DEB.2.20.1701271145260.3469@virtualbox","threadId":"41240","inReplyTo":"CAGZ79kYLFJYPQu5KSv3hG+_eavO9BHkxHjpVOEs63Nn6Hu1gTg@mail.gmail.com","subject":"Re: [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T10:49:57Z","receivedAt":"2017-01-27T10:52:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Thu, 26 Jan 2017, Stefan Beller wrote:\n\n> On Thu, Jan 26, 2017 at 8:08 AM, Johannes Schindelin\n> wrote:\n> > - if (!f)\n> > + if (!f) {\n> > + if (errno == ENOENT)\n> > + return -1;\n> > die_errno(\"Could not open file %s for reading\",\n> > git_path(\"%s\", fname));\n> \n> While at it, fix the translation with die_errno(_(..),..) ?\n\nThat is not the purpose of my patch. But feel free to offer a follow-up\npatch!\n\nCiao,\nJohannes\n"}]}