[augeas-devel] [PATCH] Add --autosave option to augtool (fix ticket #172). Define AUGTOOL_PROMPT as a constant. Do not print (null) lines when using --echo.

[Raphaël Pinson]

On Fri, Jan 7, 2011 at 11:50 PM, David Lutterkort <lutter at redhat.com> wrote:
> ACK, with some minor changes (see below)
>
> Looking through main_loop, it's starting to get pretty hard to figure
> out what's going on. Part of it is that we have 3 different ways to read
> input lines. That should be attacked with a different patch, but the
> cleanup would work along the following lines:
>
>      * when an explicit input file is given via -f, use
>        freopen(inputfile, "r", stdin) to open it; that eliminates
>        distinctions between reading from fp and from stdin
>      * try to always use readline; before entering the while(1) loop,
>        determine if we need a prompt or not, and set a new variable
>        const char *prompt to AUGTOOL_PROMPT or NULL, and then call
>        readline(prompt) inside the loop
>


OK, so after I commit this patch, I should rework the main_loop to do
that. Maybe a wrapper function to read lines would help?


>
> On Wed, 2011-01-05 at 12:52 +0100, raphink at gmail.com wrote:
>> From: Raphaël Pinson <raphink at gmail.com>
>>
>> ---
>>  src/augtool.c |   38 ++++++++++++++++++++++++++++++--------
>>  1 files changed, 30 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/augtool.c b/src/augtool.c
>> index 303e765..dbf48e2 100644
>> --- a/src/augtool.c
>> +++ b/src/augtool.c
>> @@ -44,6 +44,7 @@ char *loadpath = NULL;
>>  const char *inputfile = NULL;
>>  int echo = 0;
>>  bool print_version = false;
>> +bool auto_save = false;
>>
>>  /*
>>   * General utilities
>> @@ -79,6 +80,8 @@ struct command_opt_def {
>>
>>  #define CMD_OPT_DEF_LAST { .type = CMD_NONE, .name = NULL }
>>
>> +#define AUGTOOL_PROMPT "augtool> "
>> +
>>  /* Handlers return one of these */
>>  enum command_result {
>>      CMD_RES_OK,
>> @@ -1023,6 +1026,7 @@ static void usage(void) {
>>      fprintf(stderr, "  -I, --include DIR  search DIR for modules; can be given mutiple times\n");
>>      fprintf(stderr, "  -e, --echo         echo commands when reading from a file\n");
>>      fprintf(stderr, "  -f, --file FILE    read commands from FILE\n");
>> +    fprintf(stderr, "  --autosave         automatically save at the end of instructions\n");
>
> I think we should also offer a short option "-s" for this functionality.
>

OK, I had put one initially, but then I thought I'd do it the way you
did with --nostdinc and --noautoload. That's easy enough to fix
though.


>> @@ -1152,6 +1161,7 @@ static int main_loop(void) {
>>      size_t len = 0;
>>      char inputline [128];
>>      enum command_result code;
>> +    bool end_reached = false;
>>
>>      FILE *fp;
>>      if (inputfile) {
>> @@ -1171,19 +1181,31 @@ static int main_loop(void) {
>>                  line = inputline;
>>              }
>>              if (echo)
>> -                printf("augtool> %s", line);
>> +                printf("%s%s", AUGTOOL_PROMPT, line);
>
> Dirty C tricks: you can actually have the preprocessor concatenate
> strings; the above can also be written as
>
>        printf(AUGTOOL_PROMPT "%s", line);
>
> Not that it makes much of a difference.

If you prefer it this way, that's fine by me :-)


>
>> } else if (isatty(fileno(stdin))) {
>> -            line = readline("augtool> ");
>> +            line = readline(AUGTOOL_PROMPT);
>>          } else {
>>              if (getline(&line, &len, stdin) == -1)
>> -                return ret;
>> -            if (echo)
>> -                printf("augtool> %s", line);
>> +                line = NULL;
>> +            if (echo && line != NULL)
>> +                printf("%s%s", AUGTOOL_PROMPT, line);
>>          }
>>
>> -        cleanstr(line, '\n');
>>          if (line == NULL) {
>> -            printf("\n");
>> +            if (auto_save) {
>> +                line = "save";
>
> This causes compilation to fail for me [1], since we're assigning a
> const char * to a char *.

Funny, I didn't get a compilation error, although I do see why it
would^Wshould fail.

>
> One way to fix this is to write instead
>                strncpy(inputline, "save", sizeof(inputline));
>                line = inputline;

Isn't it possible to do

     strncpy(line, "save", sizeof(line));

directly?


Raphaël