[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fix automatic formating in seq
From: |
Jim Meyering |
Subject: |
Re: [PATCH] Fix automatic formating in seq |
Date: |
Mon, 09 Jul 2007 12:32:12 +0200 |
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