Hello,
- for (step = initialize_steps; *step != '\0'; step++)
+ for (const char *step = initialize_steps; *step != '\0'; step++)
But I still wonder why we should apply such change here.
Because it removes one declaration and reduces the scope of one variable?
If there is the reason why this change is necessary here,
Nope, such changes are never necessary.
I'm OK with that. But if not, basically I'd like to avoid the change.
Otherwise it may make the back-patch a bit harder
when we change the surrounding code.
I think that this is small enough so that it can be managed, if any back
patch occurs on the surrounding code, which is anyway pretty unlikely.
Attached is the slightly updated version of the patch. Based on your
patch, I added the descriptions about logging of "g" and "G" steps into
the doc, and did some cosmetic changes. Barrying any objections,
I'm thinking to commit this patch.
I'd suggest:
"to print one message each ..." -> "to print one message every ..."
"to print no progress ..." -> "not to print any progress ..."
I would not call "fprintf(stderr" twice in a row if I can call it once.
While reviewing the patch, I found that current code allows space
character to be specified in -I. That is, checkInitSteps() accepts
space character. Why should we do this?
Probably I understand why runInitSteps() needs to accept space character
(because "v" in the specified string with -I is replaced with a space
character when --no-vacuum option is given).
Yes, that is the reason, otherwise the string would have to be shifted.
But I'm not sure why that's also necessary in checkInitSteps(). Instead,
we should treat a space character as invalid in checkInitSteps()?
I think that it may break --no-vacuum, and I thought that there may be
other option which remove things, eventually. Also, having a NO-OP looks
ok to me.
--
Fabien.
