Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> > +static char **read_author_script(void)
>> > +{
>> > + struct strbuf script = STRBUF_INIT;
>> > + int i, count = 0;
>> > + char *p, *p2, **env;
>> > + size_t env_size;
>> > +
>> > + if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
>> > + return NULL;
>> > +
>> > + for (p = script.buf; *p; p++)
>> > + if (skip_prefix(p, "'\\\\''", (const char **)&p2))
>> > + strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
>> > + else if (*p == '\'')
>> > + strbuf_splice(&script, p-- - script.buf, 1, "", 0);
>> > + else if (*p == '\n') {
>> > + *p = '\0';
>> > + count++;
>> > + }
>>
>> Hmph. What is this loop doing? Is it decoding a sq-quoted buffer
>> or something? Don't we have a helper function to do that?
>
> Well, it is not just decoding an sq-quoted buffer, but several lines with
> definitions we sq-quoted ourselves, individually.
>
> The quote.[ch] code currently has no code to dequote lines individually.
There is a function with exactly the same name in builtin/am.c and I
assume that it is reading from a file with the same format, which
uses a helper to read one variable line at a time. Hopefully that
can be refactored so that more parsing is shared between the two
users.
Two functions with the same name reading from the same format, even
when they expect to produce the same result in different internal
format, without even being aware of each other is a bad enough
"regression" in maintainability of the code. One of them not even
using sq_dequote() helper and reinventing is even worse.