Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 06.10.2016 um 15:08 schrieb Johannes Schindelin:
> Hi Junio,
> 
> On Mon, 12 Sep 2016, Junio C Hamano wrote:
> 
>> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>>
>>>> I do not offhand see why we want to be lenient here,
>>>> especially only to the left.
>>>
>>> Postel's Law.
>>
>> How would that compare/relate to yagni, though?
> 
> I did need it, though. Blame it on being overworked or simply tired: I
> ended up inserting a spurious space before a "fixup" command. This command
> was handled as intended by the scripted rebase -i, and with the patch in
> question the rebase--helper agreed, too.
> 
> Note: we have no problem to the right because we do handle variable-length
> whitespace between command and SHA-1, and we do not care one iota about
> the exact form of the oneline (hence right-stripping is not necessary).

The last sentence is not entirely correct. See the patch below. You
may pick up the idea in one form or another, or just queue the patch
near the end of your series.

Let me take the opportunity to say

   Thank You Very Much!

for the new implementation of rebase -i. I'm using your branch
rebase-i-extra, and it is such a pleasure to work with on Windows
compared to the old fully scripted version.

---- 8< ----
[PATCH] sequencer: strip CR from the end of exec insns

It is not unheard of that editors on Windows write CRLF even if the file
originally had only LF. This is particularly awkward for exec lines of a
rebase -i todo sheet. Take for example the insn "exec echo": The shell
script parser (either of the sequencer or of the shell that is invoked,
I do not know) splits at the LF and leaves the CR attached to "echo",
which leads to the unknown command "echo\r".

Work it around by stripping CR before the command specified with exec is
passed to the shell.

This happens to fix t9903.14 and .15 in my environment: the insn sheet
constructed here contains CRLF thanks to MSYS1's bash cleverness.

Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
---
 sequencer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index daf8f13..6961820 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2005,8 +2005,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 		}
 		else if (item->command == TODO_EXEC) {
 			char *end_of_arg = (char *)(item->arg + item->arg_len);
-			int saved = *end_of_arg;
+			int saved;
 
+			while (end_of_arg != item->arg && end_of_arg[-1] == '\r')
+				end_of_arg--;
+			saved = *end_of_arg;
 			*end_of_arg = '\0';
 			res = do_exec(item->arg);
 			*end_of_arg = saved;
-- 
2.10.0.343.g37bc62b




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]