Re: [PATCH v3 2/2] Fix use of strategy options with interactive rebases
- From
Elijah Newren <newren@gmail.com>
- Date
- Jul 13, 2018, 15:51 UTC
- Message-ID
- <20180713155110.21627-1-newren@gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.1807121740270.75@tvgsbejvaqbjf.bet>
Hi Dscho,
On Thu, Jul 12, 2018 at 8:41 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi Elijah, > > On Wed, 27 Jun 2018, Elijah Newren wrote: >
...
Show 37 quoted lines
>> diff --git a/git-rebase.sh b/git-rebase.sh
>> index 19bdebb480..f3b10c7f62 100755
>> --- a/git-rebase.sh
>> +++ b/git-rebase.sh
>> @@ -328,7 +328,7 @@ do
>> do_merge=t
>> ;;
>> --strategy-option=*)
>> - strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}")"
>> + strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}" | sed -e s/^.//)"
>
> Didn't you mean to use "s/^ *//" instead?
>
>> do_merge=t
>> test -z "$strategy" && strategy=recursive
>> ;;
>> diff --git a/sequencer.c b/sequencer.c
>> index 5354d4d51e..ef9237c814 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -2206,6 +2206,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
>> static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
>> {
>> int i;
>> + char *strategy_opts_string;
>>
>> strbuf_reset(buf);
>> if (!read_oneliner(buf, rebase_path_strategy(), 0))
>> @@ -2214,7 +2215,11 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
>> if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))
>> return;
>>
>> - opts->xopts_nr = split_cmdline(buf->buf, (const char ***)&opts->xopts);
>> + strategy_opts_string = buf->buf;
>> + if (*strategy_opts_string == ' ')
>
> I think that this would ideally even be a `while` instead of an `if`.Thanks for taking a look; both sound like good suggestions. Since the patch in question has already reached next, here's a patch on top of en/rebase-i-microfixes to make these two changes:
-- 8< -- Subject: [PATCH] Whitespace handling improvements with interactive rebase strategy options
In commit 0060041df ("Fix use of strategy options with interactive rebases", 2018-06-27), extra whitespace was removed from a generated string to fix up parsing. Instead of assuming one extra space, though, we can just remove all leading whitespace and make the code slightly more robust.
Suggested-by: Johanness Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> --- git-rebase.sh | 2 +- sequencer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh index f3b10c7f62..e572980bbc 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -328,7 +328,7 @@ do do_merge=t ;; --strategy-option=*) - strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}" | sed -e s/^.//)" + strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}" | sed -e "s/ *//")" do_merge=t test -z "$strategy" && strategy=recursive ;; diff --git a/sequencer.c b/sequencer.c index ef9237c814..3f780f8f50 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2216,7 +2216,7 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) return; strategy_opts_string = buf->buf; - if (*strategy_opts_string == ' ') + while (*strategy_opts_string == ' ') strategy_opts_string++; opts->xopts_nr = split_cmdline(strategy_opts_string, (const char ***)&opts->xopts);
-- 2.18.0.645.g72fe132ec2