Re: [GSoC][PATCHl 1/6] rebase -i: add --ignore-whitespace flag

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 7, 2019, 05:38 UTC
Message-ID: <xmqqimr9ilha.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190806173638.17510-2-rohit.ashiwal265@gmail.com>

Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:

Show 8 quoted lines

> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index db6ca9bd7d..3c195ddc73 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> ...
> @@ -511,6 +523,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, NULL, options,
>  			builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0);

Curious. Did you rebase onto an older codebase? I think the round currently queued in my tree already has c0e78f7e46 which merged the jk/unused-params-final-batch topic that updated this call to parse_options() to pass prefix. Perhaps you want to fetch from me what has been on 'pu' (it should be "log master..10827432") and compare what you had? Unless there is no compelling reason not to, it would be a good idea to base the reroll on the same commit as the commit on which the previous version has been queued, which in this case is 9c9b961d ("The sixth batch", 2019-07-19).

Show 14 quoted lines

> diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh
> new file mode 100755
> index 0000000000..e617d3150e
> --- /dev/null
> +++ b/t/t3433-rebase-options-compatibility.sh
> @@ -0,0 +1,65 @@
> ...
> +
> +	git checkout --orphan master &&
> +	cat >file <<-EOF &&
> +	line 1
> +	        line 2
> +	line 3
> +	EOF

The second line triggers "indent with spaces" whitespace error; you can protect these spaces in the leading indent like so:

	sed -e "s/^|//" <<-\EOF &&
	|line 1
	|        line 2
	|line 3
	EOF

Also make it a habit to always quote the EOF token that begins the here document (i.e. <<-\EOF) when the here document does not need variable interpolation. I suspect (but I did not read all of the tests) that there may be many other instances of here document in this patch that can use the same improvement.

Thanks.