hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.11k stars 362 forks source link

Add support for binary integer values #236

Open cozzyd opened 6 months ago

cozzyd commented 6 months ago

This adds support for binary integers to the libconfig grammar, specified by a 0b or 0B prefix. These are treated very similarly to how hex integers are treated (including defining an integer format for them so they may be output, and having an L suffix for 64-bit integers). One might accuse me of grepping for hex and then doing the same thing for binary...

A few salient points:

cozzyd commented 5 months ago

Thanks for the positive reception!

Do you know yet what the next version will be (so that the version adding the feature can be documented)?

Also, I noticed that the documentation implies that values outside a 32-bit range will automatically be promoted to 64-bit, but as far as I can tell this is not the case for hex. Is this a bug or a conscious decision (maybe because of the MINGW32 workaround potentially causing some degradation in the 32-bit case?). I copied the same behavior as hex for binary, but automatic promotion to 64-bits would probably be better (and also more consistent with documentation), so I'll go ahead and implement that unless you have an objection?

hyperrealm commented 5 months ago

It's probably a bug, so feel free to fix that. All 32- and 64-bit integer values should behave the same way regardless of display format.

I don't know if that strotull() bug still exists in MinGW...I have not worked on the Windows port in several years so it may have been fixed by now.

Next version will be 1.7.4. I can prepare a release after you merge your changes. We're long overdue for a new release as a few PRs have been merged since the last one.

On Mon, Apr 15, 2024 at 10:53 PM Cosmin Deaconu @.***> wrote:

Thanks for the positive reception!

Do you know yet what the next version will be (so that the version adding the feature can be documented)?

Also, I noticed that the documentation implies that values outside a 32-bit range will automatically be promoted to 64-bit, but as far as I can tell this is not the case for hex. Is this a bug or a conscious decision (maybe because of the MINGW32 workaround potentially causing some degradation in the 32-bit case?). I copied the same behavior as hex for binary, but automatic promotion to 64-bits would probably be better (and also more consistent with documentation), so I'll go ahead and implement that unless you have an objection?

— Reply to this email directly, view it on GitHub https://github.com/hyperrealm/libconfig/pull/236#issuecomment-2058225336, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR4MIYV7L66A22J5BPQW6LY5SVFPAVCNFSM6AAAAABFUSB6PWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGIZDKMZTGY . You are receiving this because you commented.Message ID: @.***>

--

Mark Lindner http://www.hyperrealm.com/

cozzyd commented 5 months ago

I've gone ahead and tried to make all the integer parsing consistent, including automatic promotion to 64 bits and error detection. This passes tests, but probably needs to be sanity checked a bit more to make sure I didn't introduce any silly bugs. I have also taken a stab at updating the docs.