add option to statically disable decimal mode

[omarandlorraine]

So I've never looked at conditional compilation before in Rust. Maybe I got something wrong here. But the tests still pass.

[mre]

the cfg! macro checks for the condition at runtime. Did you intend to do that? There's also a config attribute for compile-time checks (https://doc.rust-lang.org/rust-by-example/attribute/cfg.html). On a more general note, how about we put the setting behind a feature flag? This way, library users can decide to enable it if they like. The syntax is quite similar.

[omarandlorraine]

I did not intend to check that flag at runtime! Thanks for the catch. I'm learning about conditional compilation in Rust as I go here so bear with.

[omarandlorraine]

Okay, CI is failing but it's nothing to do with this code I've just changed. I'll take a look in the morning.

[omarandlorraine]

Here, the tests pass again. I pushed a commit to master and then rebased this branch; hope that's okay

(It looks to me like the issue is a lint was added to clippy since the last commit to master)

[omarandlorraine]

Here, I think this branch is ready. What do you think?

[mre]

From my experience, features are usually phrased "permissively", so I'd change the feature name to just decimal_mode.
Then

#[cfg(not(feature = "disable_decimal_mode"))]

becomes

#[cfg(feature = "decimal_mode")]

At a later point in time we could consider grouping those features into more generic "emulation modes" — one for NMOS and one for CMOS for example. We could add the missing instructions for CMOS then as well. Let's keep it simple for now, though. 😉

[mre]

Oh and if we want to enable decimal mode by default, we can add it to the list of default features:

default = ["decimal_mode"]

[omarandlorraine]

Oh I see, that's better yes I agree.

But how do we put in cargo test -F something to disable it then? Or in a host project's Cargo.toml?

[mre]

To disable default features, you can pass

cargo test --no-default-features

As described in https://doc.rust-lang.org/cargo/commands/cargo-test.html#feature-selection.
So in our case we'd run cargo test for the default version with decimal support and cargo test --no-default-features for the vanilla version without decimal support.
Does that answer your question? 🤔

[omarandlorraine]

I suppose so, but in the future we'd want something like default = ["illegal-opcodes" "decimal-mode"]. And then we'd want to test all default features except decimal mode.

[mre]

Ah, the common way is to first disable all default features and then re-enabling the ones under test, e.g.

cargo test --no-default-features -F illegal-opcodes

The same is done in the Cargo.toml of some popular projects like ripgrep here.

Granted, it gets a little tedious with many features, but it's mostly a CI thing as normal users would probably just enable one feature in the end — we can combine features into more general features to make that easier if we like. 😉

If we get to a point where testing all combinations gets awkward, we can list them in a Makefile target, but let's wait until we get there.

[omarandlorraine]

How's this?

[mre]

Great, looks good to me. Thanks a lot @omarandlorraine.