Re: [PATCH] Fix automatic formating in seq

[Jim Meyering]

Pádraig Brady <address@hidden> wrote:
> While I was looking at the floating point rounding issues in seq
> a couple of weeks ago, I noticed some anomalies with the width
> automatically determined for floating point numbers,
> and even for some integer number formats.
> The changes this patch makes can be seen in the following
> before and after examples.

Wow.  Thanks for all the testing and the patch.

...
> +  //We don't output ' ' or '+' so don't include in width
> +  arg += strspn (arg, " +");

Please use C-style (not C++ //) comments.  E.g.,
/* seq doesn't output ' ' or '+' so don't include in width.  */

However, using strspn that way makes seq accept invalid input
like "++++   ++ + +1".  This is probably what you want (accepting
leading TABs, too):

  arg += strspn (arg, " \t");
  arg += *arg == '+';

>    ret.width = strlen (arg);
>    ret.precision = INT_MAX;
>
> -  if (! arg[strcspn (arg, "eExX")] && isfinite (ret.value))
> +  if (! arg[strcspn (arg, "xX")] && isfinite (ret.value))
>      {
>        char const *decimal_point = strchr (arg, '.');
>        if (! decimal_point)
>       ret.precision = 0;
>        else
>       {
> -       size_t fraction_len = strlen (decimal_point + 1);
> +       size_t fraction_len = strcspn (decimal_point+1, "eE");
>         if (fraction_len <= INT_MAX)
>           ret.precision = fraction_len;
> -       ret.width += (fraction_len == 0
> +       ret.width += (fraction_len == 0                      //#.      -> #
>                       ? -1
> -                     : (decimal_point == arg
> -                        || ! ISDIGIT (decimal_point[-1])));
> +                     : (decimal_point == arg                //.#      -> 0.#
> +                        || ! ISDIGIT (decimal_point[-1]))); //[ +-].# -> 0.#
> +     }
> +      char const *e = strchr(arg, 'e');
> +      if (! e) e = strchr(arg, 'E');
> +      if (e)
> +     {
> +       long exponent = strtol (e+1, NULL, 10);

strtol can fail.
Use xstrtol instead.

Would you please write ChangeLog entries for this?

----------------------------
I'm glad to see you're using the git repository, now :-)
Here's a minor procedural request.  Patches generated this way
are easier for me to apply, and more reliable too.

Here's one way to do it:

  # Check out a copy of the repository into a dir named "cu".
  git clone git://git.sv.gnu.org/coreutils cu
  cd cu

  # Create a private branch named seq-improvement (or whatever you want).
  # This is where you'll make and commit any seq-related changes.
  # Your commits are local to your repository and this private branch.
  # Note: don't run "git pull" when on this branch.
  git checkout -b seq-improvement

  # apply your patch, then commit it, using a ChangeLog-style log entry
  # where the first line is a one-line summary.
  patch -p0 ...
  git commit

  # Then, create a patch file you can mail to this list:
  git format-patch --stdout --signoff HEAD~1 > seq-patch

  # If you decide you'd rather revise your patch, one way to
  # create a new version of that "topmost" delta is to
  # change any files and then run cg-commit --amend -e.
  # This is one of the few cases where a cogito command (cg-commit)
  # is better than the git-based command: "git commit --amend -e".
  # You _can_ use the git-based one, but you'd have to first run
  # "git add FILE..." listing all files whose changes you'd like to include.

  # Browse the changes you've added to your branch using the "gitk" GUI.
  # There should be no branching.
-------------------------

If you have pending (committed) changes on your branch, yet newer
changes have been pushed to the public trunk ("master" branch),
you can update your working dir like this:

  # This should show no uncommitted changes (i.e. no output).
  # If there is any output, commit or revert those changes.
  git diff

  # This puts you back on the trunk
  git checkout master

  # Pull changes from the repository on savannah
  git pull

  # Get back on your working branch
  git checkout seq-improvement

  # Merge your work so that it applies cleanly to the trunk.
  # This is where you may encounter conflicts.
  git rebase master