Correct floating decimal point scaling of fractions in sfio library #131
Conversation
… to text conversions within sfio library.
|
Thank you for this fix, it looks good to me. I just had a few minor nits regarding whitespace formatting. Instead of making you jump through hoops I just fixed it myself (see commits above). I moved the regression tests since they didn't have much directly to do with variables. math.sh seemed like a logical place. builtins.sh would also have been logical but it's already huge... plus, if/when you submit more math-related fixes, their regression tests can go into math.sh as well. On my end, the whitespace formatting in the sfcvt.c changes was a mess. I assume it looked fine in your editor, so it looks like your editor may be set to expand tabs to spaces using a nonstandard tab width. In future would you mind making sure that you're using the standard 8-space tab with without expanding tabs to spaces? That's what the rest of this code base uses. Oh, and AT&T also has a peculiar style for shell code -- bin/package and related scripts are good examples. There are a couple of different coding styles in use (libast uses a slightly different one from ksh93 and it may also vary by author). In general it's best to follow the style of whatever file you're editing as closely as possible. I've no comments on the fix itself -- it seems to work fine. Thanks again for the PR! |
_sfcvt(), "convert a floating point value to ASCII", did not adjust for negative decimal place movement as what happens with leading zeroes. This caused ksh's 'printf %f' formatter to fail to round floating point values correctly. src/lib/libast/sfio/sfcvt.c: - Removed constraint of <1e-8 for doubles by matching what was done for long doubles having <.1. - Corrected a condition when the next power of 10 occurred and that new 1 digit was being overwritten by a 0. src/cmd/ksh93/tests/math.sh: - Validate that typeset -E/F formatting matches that of their equivalent printf formatting options as well as checking for correct float scaling of the fractional parts.
|
You are most welcome. As this was my first ever pull request, I feel a lot of satisfaction in giving back to ksh and the open source community. I do appreciate you taking the time and cleaning up the code I submitted as you are right that my editor uses 4 spaces for tabs as I more than likely got that from Python guidelines. As this is not Python, I will strive to match the existing code to the best of my abilities as you have described. I am learning every day and should hopefully submit better coded PRs in the future. |
Here are the necessary changes to correct the scaling of floating point decimal values with fractions. sfcvt.c did not adjust for negative decimal place movement as what happens with leading zeroes. Removed constraint of
<1e-8for doubles by matching what was done for long doubles having<.1. Corrected a condition when the next power of 10 occurred and that new 1 digit was being overwritten by a 0.