{"thread":{"id":"39535","subject":"Suggestion: add author info to TODO list in git-rebase--interactive","startedAt":"2015-06-05T19:00:17Z","lastAt":"2015-06-07T06:14:27Z","messageCount":5,"participants":["Mike Rappazzo","Junio C Hamano","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"263052","messageId":"CANoM8SWX+UVWdV-Aeqqn8DuMf74zUWx=rbEaz3ctNDwa_ZZbeg@mail.gmail.com","threadId":"39535","inReplyTo":null,"subject":"Suggestion: add author info to TODO list in git-rebase--interactive","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-05T19:00:17Z","receivedAt":"2015-06-05T19:00:17Z","isPatch":false,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"I find that If I am doing a rebase with the intention to squash or\nre-order commits, it is helpful to know the commit author.\n\nHowever, the alteration that I have made to git-rebase--interactive\nmay not be entirely correct.  Here is the change:\n\n---\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex dc3133f..ec44d41 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -977,7 +977,7 @@ else\n        revisions=$onto...$orig_head\n        shortrevisions=$shorthead\n fi\n-git rev-list $merges_option --pretty=oneline --reverse --left-right\n--topo-order \\\n+git rev-list $merges_option --pretty=\"%m%h [%an] %x09%s\" --reverse\n--left-right --topo-order \\\n        $revisions ${restrict_revision+^$restrict_revision} | \\\n        sed -n \"s/^>//p\" |\n while read -r sha1 rest\n--\n2.4.2\n\n\nThe problem, as I see it is that the original '--pretty=oneline' only\nproduces a single line of output (of course).  However, the changed\nversion '--pretty=\"%m%h [%an] %x09%s\"' produces multiple lines.  The\ncommand seems to ignore the unimportant lines, and the expected output\nis put into the TODO list though.\n\nIs there a better way of providing this information, or is this still\nacceptable?\n\n_Mike\n"},{"id":"263056","messageId":"xmqq8ubyrlsc.fsf@gitster.dls.corp.google.com","threadId":"39535","inReplyTo":"CANoM8SWX+UVWdV-Aeqqn8DuMf74zUWx=rbEaz3ctNDwa_ZZbeg@mail.gmail.com","subject":"Re: Suggestion: add author info to TODO list in git-rebase--interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-05T19:35:15Z","receivedAt":"2015-06-05T19:35:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo <rappazzo@gmail.com> writes:\n\n> I find that If I am doing a rebase with the intention to squash or\n> re-order commits, it is helpful to know the commit author.\n\nThere is not a fundamental reason why the remainder of the line\nafter the object name in the rebase insn sheet should not be\ncustomizable, and I think your patch is a good first step to\nidentify where that customization should go.\n\nBut that is a customization issue, not changing the default and the\nonly format used.\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index dc3133f..ec44d41 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -977,7 +977,7 @@ else\n>         revisions=$onto...$orig_head\n>         shortrevisions=$shorthead\n>  fi\n> -git rev-list $merges_option --pretty=oneline --reverse --left-right\n> --topo-order \\\n> +git rev-list $merges_option --pretty=\"%m%h [%an] %x09%s\" --reverse\n> --left-right --topo-order \\\n>         $revisions ${restrict_revision+^$restrict_revision} | \\\n>         sed -n \"s/^>//p\" |\n>  while read -r sha1 rest\n\nThis is nothing new, and it may not even be a problem, but why do we\nuse --left-right and then filter with sed?  Does this part of the\ncode predate --left-only, I wonder.  I'd probably write\n\n\tgit log --format=\"%h\t%s\" --left-only .....\n\nwith today's Git, but perhaps I am missing something?\n\n> The problem, as I see it is that the original '--pretty=oneline' only\n> produces a single line of output (of course).  However, the changed\n> version '--pretty=\"%m%h [%an] %x09%s\"' produces multiple lines.\n\nShouldn't you be using tformat, not format, if you are doing a\noneline emulation?\n"},{"id":"263057","messageId":"CAPig+cS0dW=KpD3C_6ugiJTccQqNu1NopcU6Pc71hf6iLLbcfw@mail.gmail.com","threadId":"39535","inReplyTo":"xmqq8ubyrlsc.fsf@gitster.dls.corp.google.com","subject":"Re: Suggestion: add author info to TODO list in git-rebase--interactive","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-05T19:39:32Z","receivedAt":"2015-06-05T19:39:32Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 5, 2015 at 3:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mike Rappazzo <rappazzo@gmail.com> writes:\n>> I find that If I am doing a rebase with the intention to squash or\n>> re-order commits, it is helpful to know the commit author.\n>\n> There is not a fundamental reason why the remainder of the line\n> after the object name in the rebase insn sheet should not be\n> customizable, and I think your patch is a good first step to\n> identify where that customization should go.\n>\n> But that is a customization issue, not changing the default and the\n> only format used.\n\nThe idea of being able to provide a custom format for insn sheet lines\ncame up within the last year and was somewhat more well-developed and\na bit more heavily discussed. I don't recall whether there was an\naccompanying patch, and I am unfortunately unable to locate the\ndiscussion in the archive.\n"},{"id":"263079","messageId":"CANoM8SXnTU5bnyLKUUYEtnug89cC9cN3AU3E07qUmk85teamLw@mail.gmail.com","threadId":"39535","inReplyTo":"CAPig+cS0dW=KpD3C_6ugiJTccQqNu1NopcU6Pc71hf6iLLbcfw@mail.gmail.com","subject":"Re: Suggestion: add author info to TODO list in git-rebase--interactive","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-06T01:00:51Z","receivedAt":"2015-06-06T01:00:51Z","isPatch":false,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"I think the custom format makes sense.  I took a first pass.\n\nA config option 'rebase.interactive.todo-format' can override the\ndefault 'oneline' format of the TODO list.  Since the list is\nparsed using the left, right or boundary mark plus the sha1, then if the\ncustom format does not start with those values, they will be\nautomatically added to the beginning of the custom format.\n\nFor example, if author information is desired for the TODO list, then\nsetting the config value to '[%an] %s' will actually result in the\nformat being '%m%h [%an] %s'\n\n---\n git-rebase--interactive.sh | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex dc3133f..e2d5ffc 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -977,7 +977,18 @@ else\n  revisions=$onto...$orig_head\n  shortrevisions=$shorthead\n fi\n-git rev-list $merges_option --pretty=oneline --reverse --left-right\n--topo-order \\\n+custom_format=$(git config --get rebase.interactive.todo-format)\n+if test -z \"$custom_format\"\n+then\n+   custom_format=\"oneline\"\n+else\n+   # the custom format MUST start with %m%h or %m%H\n+ if test \"${custom_format:0:5}\" != '%m%h '\n+   then\n+      custom_format=\"%m%h ${custom_format}\"\n+   fi\n+fi\n+git rev-list $merges_option --pretty=\"${custom_format}\" --reverse\n--left-right --topo-order \\\n  $revisions ${restrict_revision+^$restrict_revision} | \\\n  sed -n \"s/^>//p\" |\n while read -r sha1 rest\n-- \n\nIs this closer to what you are looking for?  I also tried changing the\n'--left-right' to '--left-only', but that seemed to not produce any\nresults.\n\n\n\n\nOn Fri, Jun 5, 2015 at 3:39 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Fri, Jun 5, 2015 at 3:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Mike Rappazzo <rappazzo@gmail.com> writes:\n>>> I find that If I am doing a rebase with the intention to squash or\n>>> re-order commits, it is helpful to know the commit author.\n>>\n>> There is not a fundamental reason why the remainder of the line\n>> after the object name in the rebase insn sheet should not be\n>> customizable, and I think your patch is a good first step to\n>> identify where that customization should go.\n>>\n>> But that is a customization issue, not changing the default and the\n>> only format used.\n>\n> The idea of being able to provide a custom format for insn sheet lines\n> came up within the last year and was somewhat more well-developed and\n> a bit more heavily discussed. I don't recall whether there was an\n> accompanying patch, and I am unfortunately unable to locate the\n> discussion in the archive.\n"},{"id":"263136","messageId":"xmqq6170dozg.fsf@gitster.dls.corp.google.com","threadId":"39535","inReplyTo":"CANoM8SXnTU5bnyLKUUYEtnug89cC9cN3AU3E07qUmk85teamLw@mail.gmail.com","subject":"Re: Suggestion: add author info to TODO list in git-rebase--interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-07T06:14:27Z","receivedAt":"2015-06-07T06:14:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo <rappazzo@gmail.com> writes:\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index dc3133f..e2d5ffc 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -977,7 +977,18 @@ else\n>   revisions=$onto...$orig_head\n>   shortrevisions=$shorthead\n>  fi\n> -git rev-list $merges_option --pretty=oneline --reverse --left-right\n> --topo-order \\\n> +custom_format=$(git config --get rebase.interactive.todo-format)\n\nWe use three-level names only when we need an unbounded end-user\nsupplied things (e.g. nickname for remotes in \"remote.*.url\",\ndescription for a branch in \"branch.*.description\").  \"interactive\"\nis not such a token, so a two-level name, e.g. \"rebase.insnformat\"\nor something like that.  Also core Git avoids variable names with\n\"-\" in them.\n\n> +if test -z \"$custom_format\"\n> +then\n> +   custom_format=\"oneline\"\n> +else\n> +   # the custom format MUST start with %m%h or %m%H\n> + if test \"${custom_format:0:5}\" != '%m%h '\n> +   then\n> +      custom_format=\"%m%h ${custom_format}\"\n> +   fi\n> +fi\n\nWhy not allow them to *ONLY* set what follows '%m%h '?  That is, if\nthey say '%m%h %s', give them '%m%h %m%h %s', by unconditionally\nprepend '%m%h '.  That way you do not need these conditional.\n\nSomething along the lines of...\n\n        format=$(git config rebase.insnFormat)\n        if test -z \"$format\"\n        then\n                format=\"%m%h\t%s\"\n        else\n                format=\"%m%h\t$format\"\n        fi\n\n        git rev-list $merge_option --format=\"$format\" --reverse \\\n                --topo-order --left-right \\\n                ...\n\n> ...  I also tried changing the\n> '--left-right' to '--left-only', but that seemed to not produce any\n> results.\n\nWouldn't we want right side of the symmetric difference, though?\n"}]}