[bug-cppi] Re: [PATCH] Add new option --in-place

[Giuseppe Scrivano]

Ping.



Giuseppe Scrivano <address@hidden> writes:

> Hello,
>
> what do you think about the following patch?  I was already using cppi
> trough a shell script to write to a temporary file and after copy it on
> the original file, looping on a collection of files.  The patch
> simplifies the task and allow cppi to be used as "cppi -i *.c".
>
> Cheers,
> Giuseppe
>
>
>
>
> From 0cfeec83689e0b9f33f489aecef4d0b12cb20a7f Mon Sep 17 00:00:00 2001
> From: Giuseppe Scrivano <address@hidden>
> Date: Fri, 5 Mar 2010 02:19:19 +0100
> Subject: [PATCH] Add new option --in-place
>
> * bootstrap.conf (gnulib_modules): Add `full-write', `safe-read',
> `tempname' and `unlink' gnulib modules.
> * src/cppi.l: Include "full-write.h", "safe-read.h", "tempname.h".
> (long_options): Add entry for --in-place.
> (main): Handle --in-place.
> * tests/test-common: Test the --in-place option.
> ---
>  bootstrap.conf    |    4 +++
>  src/cppi.l        |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/test-common |   13 ++++++++-
>  3 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 131f358..dcd0ab0 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -31,6 +31,7 @@ gnulib_modules="
>    do-release-commit-and-tag
>    error
>    exitfail
> +  full-write
>    getopt
>    git-version-gen
>    gitlog-to-changelog
> @@ -46,10 +47,13 @@ gnulib_modules="
>    quote
>    quotearg
>    realloc
> +  safe-read
>    stat-macros
>    stdbool
>    stpcpy
> +  tempname
>    timespec
> +  unlink
>    unlocked-io
>    update-copyright
>    useless-if-before-free
> diff --git a/src/cppi.l b/src/cppi.l
> index f46940a..9d1a165 100644
> --- a/src/cppi.l
> +++ b/src/cppi.l
> @@ -45,8 +45,11 @@
>  
>  #include "cpp.h"
>  #include "error.h"
> +#include "full-write.h"
>  #include "obstack.h"
> +#include "safe-read.h"
>  #include "xstrtol.h"
> +#include "tempname.h"
>  
>  /* The official name of this program (e.g., no `g' prefix).  */
>  #define PROGRAM_NAME "cppi"
> @@ -129,6 +132,7 @@ static struct option const long_options[] =
>  {
>    {"ansi", no_argument, NULL, 'a'},
>    {"check", no_argument, NULL, 'c'},
> +  {"in-place", no_argument, NULL, 'i'},
>    {"list-files-only", no_argument, NULL, 'l'},
>    {"max-string-length", required_argument, NULL, 'm'},
>    {GETOPT_HELP_OPTION_DECL},
> @@ -659,6 +663,7 @@ With no FILE, or when FILE is -, read standard input.\n\
>  \n\
>    -a, --ansi             when checking, fail if text follows #else or 
> #endif\n\
>    -c, --check            set exit code, but don't produce any output\n\
> +  -i, --in-place         edit files in place\n\
>    -l, --list-files-only  don't generate diagnostics about indentation;\n\
>                           print to stdout only the names of files that\n\
>                           are not properly indented\n\
> @@ -785,12 +790,13 @@ main (int argc, char** argv)
>    char **file_list;
>    int c;
>    int fail = 0;
> +  bool in_place = false;
>  
>    set_program_name (argv[0]);
>  
>    atexit (close_stdout);
>  
> -  while ((c = getopt_long (argc, argv, "aclm:", long_options, NULL)) != -1)
> +  while ((c = getopt_long (argc, argv, "acilm:", long_options, NULL)) != -1)
>      {
>        switch (c)
>          {
> @@ -805,6 +811,10 @@ main (int argc, char** argv)
>            inhibit_output = 1;
>            break;
>  
> +        case 'i':
> +          in_place = true;
> +          break;
> +
>          case 'l':
>            inhibit_output = 1;
>            list_files_only = 1;
> @@ -832,18 +842,41 @@ main (int argc, char** argv)
>    if (fail)
>      exit (EXIT_FAILURE);
>  
> -  if (!inhibit_output && argc - optind > 2)
> +  if (!in_place && !inhibit_output && argc - optind > 2)
>      {
>        error (0, 0, _("too many arguments"));
>        usage (EXIT_FAILURE);
>      }
>  
> +  if (in_place && (inhibit_output || ansi_check || list_files_only))
> +    {
> +      error (0, 0, _("-i is ignored, it can't be used with -a, -c or -l"));
> +      in_place = false;
> +    }
> +
>    file_list = (optind == argc ? default_file_list : argv + optind);
>  
>    max_err = 0;
>    for (i = 0; file_list[i]; i++)
>      {
>        int err;
> +      /* Used when in_place is true.  */
> +      char tmp_filename[16];
> +
> +      if (in_place)
> +        {
> +          int dest_fileno;
> +          strcpy (tmp_filename, "./.cppiXXXXXX");
> +
> +          dest_fileno = gen_tempname (tmp_filename, 0, 0, GT_FILE);
> +          if (dest_fileno < 0)
> +            error (EXIT_FAILURE, errno, _("cannot create temporary file"));
> +          close (dest_fileno);
> +
> +          yyout = fopen (tmp_filename, "wb");
> +          if (yyout == NULL)
> +            error (EXIT_FILE_ERROR, errno, "%s", tmp_filename);
> +        }
>  
>        err = cpp_indent (file_list[i]);
>        if (err > max_err)
> @@ -851,7 +884,43 @@ main (int argc, char** argv)
>  
>        if (err && list_files_only)
>          puts (file_list[i]);
> +
> +      if (in_place)
> +        {
> +          int src_fd, dest_fd;
> +          char buf[2048];
> +
> +          fclose (yyout);
> +
> +          src_fd = open (tmp_filename, O_RDONLY | O_BINARY);
> +          if (src_fd < 0)
> +            error (EXIT_FAILURE, errno, _("cannot open source file"));
> +
> +          dest_fd = open (file_list[i], O_WRONLY | O_BINARY | O_TRUNC);
> +          if (dest_fd < 0)
> +            error (EXIT_FAILURE, errno, _("cannot open destination file"));
> +
> +          for (;;)
> +            {
> +              size_t n_read = safe_read (src_fd, buf, sizeof (buf));
> +              if (n_read == SAFE_READ_ERROR)
> +                error (EXIT_FAILURE, errno, _("error reading \"%s\""),
> +                       tmp_filename);
> +              if (n_read == 0)
> +                break;
> +
> +              if (full_write (dest_fd, buf, n_read) < n_read)
> +                error (EXIT_FAILURE, errno, _("error writing \"%s\""),
> +                       file_list[i]);
> +            }
> +
> +          close (src_fd);
> +          close (dest_fd);
> +
> +          unlink (tmp_filename);
> +        }
>      }
>  
> +
>    exit (max_err);
>  }
> diff --git a/tests/test-common b/tests/test-common
> index e393eeb..37a1f4d 100644
> --- a/tests/test-common
> +++ b/tests/test-common
> @@ -27,5 +27,16 @@ else
>    fail=1
>  fi
>  
> -test $fail = 0 && rm -f $t.I $t.O $t.e $t.EO $t.Ee $extra_temps
> +cp $t.I $t.Ii
> +cppi -i $t.Ii
> +cppi $t.I > $t.Iio
> +
> +if cmp  $t.Ii $t.Iio ; then
> +  :
> +else
> +  echo "the in-place option generated a different file"
> +  fail=1
> +fi
> +
> +test $fail = 0 && rm -f $t.I $t.O $t.e $t.EO $t.Ii $t.Iio $t.Ee $extra_temps
>  exit $fail