{"thread":{"id":"39567","subject":"[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","startedAt":"2015-06-08T21:00:01Z","lastAt":"2015-06-10T00:44:47Z","messageCount":9,"participants":["Michael Rappazzo","Junio C Hamano","Johannes Schindelin","Mike Rappazzo"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"263308","messageId":"1433797202-97425-1-git-send-email-rappazzo@gmail.com","threadId":"39567","inReplyTo":null,"subject":"[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-08T21:00:01Z","receivedAt":"2015-06-08T21:00:01Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Difference between v1 and v2 of this patch:\n\n - Fixed indentation from spaces to match the existing style\n - Changed the prepended sha1 from short (%h) to long (%H)\n - Used bash variable default when the config option is not present\n\nMichael Rappazzo (1):\n git-rebase--interactive.sh: add config option for custom instruction\n format\n\n git-rebase--interactive.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\n-- \n2.4.2\n"},{"id":"263309","messageId":"1433797202-97425-2-git-send-email-rappazzo@gmail.com","threadId":"39567","inReplyTo":"1433797202-97425-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-08T21:00:02Z","receivedAt":"2015-06-08T21:00:02Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"A config option 'rebase.instructionFormat' can override the\ndefault 'oneline' format of the rebase instruction list.\n\nSince the list is parsed using the left, right or boundary mark plus\nthe sha1, they are prepended to the instruction format.\n\nSigned-off-by: Michael Rappazzo \n---\n git-rebase--interactive.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex dc3133f..b92375e 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -977,7 +977,9 @@ else\n \trevisions=$onto...$orig_head\n \tshortrevisions=$shorthead\n fi\n-git rev-list $merges_option --pretty=oneline --reverse --left-right --topo-order \\\n+format=$(git config --get rebase.instructionFormat)\n+# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n+git rev-list $merges_option --format=\"%m%H ${format-%s}\" --reverse --left-right --topo-order \\\n \t$revisions ${restrict_revision+^$restrict_revision} | \\\n \tsed -n \"s/^>//p\" |\n while read -r sha1 rest\n-- \n2.4.2\n"},{"id":"263313","messageId":"xmqqioax99ee.fsf@gitster.dls.corp.google.com","threadId":"39567","inReplyTo":"1433797202-97425-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T21:29:13Z","receivedAt":"2015-06-08T21:29:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo writes:\n\n> A config option 'rebase.instructionFormat' can override the\n> default 'oneline' format of the rebase instruction list.\n>\n> Since the list is parsed using the left, right or boundary mark plus\n> the sha1, they are prepended to the instruction format.\n>\n> Signed-off-by: Michael Rappazzo \n> ---\n> git-rebase--interactive.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index dc3133f..b92375e 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -977,7 +977,9 @@ else\n> \trevisions=$onto...$orig_head\n> \tshortrevisions=$shorthead\n> fi\n> -git rev-list $merges_option --pretty=oneline --reverse --left-right --topo-order \\\n> +format=$(git config --get rebase.instructionFormat)\n> +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n> +git rev-list $merges_option --format=\"%m%H ${format-%s}\" --reverse --left-right --topo-order \\\n> \t$revisions ${restrict_revision+^$restrict_revision} | \\\n> \tsed -n \"s/^>//p\" |\n> while read -r sha1 rest\n\nLooks OK, but\n\n - Needs a patch to Documentation/config.txt and possibly also\n Documentation/git-rebase.txt\n\n - Needs tests somewhere in t34?? series.\n\nAlso I think \"git rev-list\" line has got way too long. As it is\nalready using backslash-continuation, do not hesitate to fold it\nfurther, e.g. \n\n git rev-list $merges_option --format=\"%m%H ${format-%s}\" \\\n --reverse --left-right --topo-order \\\n $revisions ${restrict_revision+^$restrict_revision} | \\\n sed -n \"s/^>//p\" |\n\twhile ...\n\nThanks.\n"},{"id":"263318","messageId":"xmqq38219755.fsf@gitster.dls.corp.google.com","threadId":"39567","inReplyTo":"1433797202-97425-1-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T22:17:58Z","receivedAt":"2015-06-08T22:17:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo writes:\n\n> Difference between v1 and v2 of this patch:\n>\n> - Fixed indentation from spaces to match the existing style\n> - Changed the prepended sha1 from short (%h) to long (%H)\n> - Used bash variable default when the config option is not present\n>\n> Michael Rappazzo (1):\n> git-rebase--interactive.sh: add config option for custom instruction\n> format\n>\n> git-rebase--interactive.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n\nDoes this pass tests? I see many failures including t3415.\n"},{"id":"263335","messageId":"9a61f06b14f58a0bd2b687e2aef2b805@www.dscho.org","threadId":"39567","inReplyTo":"1433797202-97425-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-06-09T09:36:57Z","receivedAt":"2015-06-09T09:36:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn 2015-06-08 23:00, Michael Rappazzo wrote:\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index dc3133f..b92375e 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -977,7 +977,9 @@ else\n> \trevisions=$onto...$orig_head\n> \tshortrevisions=$shorthead\n> fi\n> -git rev-list $merges_option --pretty=oneline --reverse --left-right\n> --topo-order \\\n> +format=$(git config --get rebase.instructionFormat)\n> +# the 'rev-list .. | sed' requires %m to parse; the instruction\n> requires %H to parse\n> +git rev-list $merges_option --format=\"%m%H ${format-%s}\" --reverse\n> --left-right --topo-order \\\n\nThese two lines are too long (longer than 80 columns)...\n\nBesides, are you sure you don't want to substitute an empty 'rebase.instructionFormat' by '%s'? I would have expected to read `${format:-%s}` (note the colon), but then, this was Junio's suggestion... Junio, what do you think, should we not rather substitute empty values by `%s` as if the config setting was unset?\n\n> \t$revisions ${restrict_revision+^$restrict_revision} | \\\n> \tsed -n \"s/^>//p\" |\n> while read -r sha1 rest\n\nCiao,\nJohannes\n"},{"id":"263340","messageId":"CANoM8SWngx-hCmNMJLZ6NhDzE=OSSH_UAd-8x7xCQZ58GzeJyw@mail.gmail.com","threadId":"39567","inReplyTo":"9a61f06b14f58a0bd2b687e2aef2b805@www.dscho.org","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-09T10:26:36Z","receivedAt":"2015-06-09T10:26:36Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"I see your point, and I'll explore that avenue.\n\nPersonally, I like the idea that one could also use the short hash if\nthe custom instruction started with \"%h \", but I see the value in\nleaving the variable blank.\n\nAfter running the tests with a custom format enabled, I did find that\nautosquash doesn't work, so I am working to correct that.\n\nOn Tue, Jun 9, 2015 at 5:36 AM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On 2015-06-08 23:00, Michael Rappazzo wrote:\n>\n>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n>> index dc3133f..b92375e 100644\n>> --- a/git-rebase--interactive.sh\n>> +++ b/git-rebase--interactive.sh\n>> @@ -977,7 +977,9 @@ 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>> +format=$(git config --get rebase.instructionFormat)\n>> +# the 'rev-list .. | sed' requires %m to parse; the instruction\n>> requires %H to parse\n>> +git rev-list $merges_option --format=\"%m%H ${format-%s}\" --reverse\n>> --left-right --topo-order \\\n>\n> These two lines are too long (longer than 80 columns)...\n>\n> Besides, are you sure you don't want to substitute an empty 'rebase.instructionFormat' by '%s'? I would have expected to read `${format:-%s}` (note the colon), but then, this was Junio's suggestion... Junio, what do you think, should we not rather substitute empty values by `%s` as if the config setting was unset?\n>\n>> $revisions ${restrict_revision+^$restrict_revision} | \\\n>> sed -n \"s/^>//p\" |\n>> while read -r sha1 rest\n>\n> Ciao,\n> Johannes\n"},{"id":"263412","messageId":"xmqqmw0865zk.fsf@gitster.dls.corp.google.com","threadId":"39567","inReplyTo":"9a61f06b14f58a0bd2b687e2aef2b805@www.dscho.org","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-09T19:23:27Z","receivedAt":"2015-06-09T19:23:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Besides, are you sure you don't want to substitute an empty\n> rebase.instructionFormat' by '%s'? I would have expected to read\n> ${format:-%s}` (note the colon), but then, this was Junio's\n> suggestion...\n\nThat was me simply being sloppy myself, expecting people not to copy\nand paste literally without thinking. Thanks for noticing.\n"},{"id":"263413","messageId":"CANoM8SVXmYJn7wbxcPuBSUa-X8v0nggpv_vC8LtPgaBwMtP0HQ@mail.gmail.com","threadId":"39567","inReplyTo":"xmqqmw0865zk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-09T19:38:54Z","receivedAt":"2015-06-09T19:38:54Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"I have since reworked this script to support the short hash in the\ncustom format as a special case:\n\n-git rev-list $merges_option --pretty=oneline --reverse --left-right\n--topo-order \\\n+format=$(git config --get rebase.instructionFormat)\n+no_format=$?\n+if test ${no_format} -ne 0\n+then\n+ format=\"%H %s\"\n+elif test \"${format:0:3}\" != \"%H \" && test \"${format:0:3}\" != \"%h \"\n+then\n+ format=\"%H ${format}\"\n+fi\n+# the 'rev-list .. | sed' requires %m to parse; the instruction\nrequires %H to parse\n+git rev-list $merges_option --format=\"%m${format}\" \\\n+ --reverse --left-right --topo-order \\\n\nI also use the $no_format variable later on in the autosquash\nre-ordering, and have the tests passing. I want to add some new tests\non the custom format, and will send a new patch when that is complete.\n\nOn Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n>\n>> Besides, are you sure you don't want to substitute an empty\n>> rebase.instructionFormat' by '%s'? I would have expected to read\n>> ${format:-%s}` (note the colon), but then, this was Junio's\n>> suggestion...\n>\n> That was me simply being sloppy myself, expecting people not to copy\n> and paste literally without thinking. Thanks for noticing.\n"},{"id":"263425","messageId":"xmqqwpzcv1c0.fsf@gitster.dls.corp.google.com","threadId":"39567","inReplyTo":"CANoM8SVXmYJn7wbxcPuBSUa-X8v0nggpv_vC8LtPgaBwMtP0HQ@mail.gmail.com","subject":"Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T00:44:47Z","receivedAt":"2015-06-10T00:44:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo writes:\n\n> I have since reworked this script to support the short hash in the\n> custom format as a special case:\n\nI thought that we always give short form when presenting it to the\nend user to edit, but for internal bookkeeping purposes we make sure\nthat we use the full SHA-1, so that new objects created during the\nrebase session will not cause \"used to be unique but not anymore\"\nambiguity in the commit object names in the instruction sheet.\n\nI have been assuming that the \"rev-list\" we have been mucking with\nwas to prepare the latter, the internal bookkeeping data, which must\nalways spell 40-hex (that is why the default \"oneline\" is not\n\"--oneline\" but \"--pretty=oneline).\n\nWhy is it necessary to make %m%H part customizable in the first\nplace?\n\nPuzzled....\n\n>\n> -git rev-list $merges_option --pretty=oneline --reverse --left-right\n> --topo-order \\\n> +format=$(git config --get rebase.instructionFormat)\n> +no_format=$?\n> +if test ${no_format} -ne 0\n> +then\n> + format=\"%H %s\"\n> +elif test \"${format:0:3}\" != \"%H \" && test \"${format:0:3}\" != \"%h \"\n\nDo not use bash-ism \"substring\".\n\n> +then\n> + format=\"%H ${format}\"\n> +fi\n> +# the 'rev-list .. | sed' requires %m to parse; the instruction\n> requires %H to parse\n> +git rev-list $merges_option --format=\"%m${format}\" \\\n> + --reverse --left-right --topo-order \\\n>\n> I also use the $no_format variable later on in the autosquash\n> re-ordering, and have the tests passing. I want to add some new tests\n> on the custom format, and will send a new patch when that is complete.\n>\n> On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano wrote:\n>> Johannes Schindelin writes:\n>>\n>>> Besides, are you sure you don't want to substitute an empty\n>>> rebase.instructionFormat' by '%s'? I would have expected to read\n>>> ${format:-%s}` (note the colon), but then, this was Junio's\n>>> suggestion...\n>>\n>> That was me simply being sloppy myself, expecting people not to copy\n>> and paste literally without thinking. Thanks for noticing.\n"}]}