Add IniBuilder

[Qard]

This might only be useful in combination with the embed feature so it may make sense to gate this behind that feature, but it doesn't depend on it in any way--it can exist without it. I left the feature-gating out, but I could change that if you prefer.

[Xenira]

Haven't had a detailed look yet. But looks good.

I think we can keep it without a feature guard for now.

[Xenira]

Would you mind adding documentation in the /guide?

And thanks again for contributing!

[Qard]

Yep, I can add a guide for that. Where do you think it should go?

[Qard]

Seems the ini builder API didn't exist until 8.2. I've added compile gates to builders/mod.rs, but I'm not sure how to do the equivalent for the header include in the wrapper.h file. 🤔

[Xenira]

Seems the ini builder API didn't exist until 8.2. I've added compile gates to builders/mod.rs, but I'm not sure how to do the equivalent for the header include in the wrapper.h file. 🤔

Guess you could pass that in here: https://github.com/davidcole1340/ext-php-rs/blob/cafaecc692c474281925716f256f0842e4580b28/build.rs#L177

See windows build: https://github.com/davidcole1340/ext-php-rs/blob/cafaecc692c474281925716f256f0842e4580b28/windows_build.rs#L53

[Qard]

I did some restructuring of the build file to include an ApiVersion enum which can be used for managing both the cfg flag names and C define names in the stacked form it was using before, so we can do EXT_PHP_RS_PHP_82 to gate C which depends on 8.2 minimum. 🙂

[ptondereau]

@Qard merged the clippy PR, you can rebase

[Qard]

Thanks! Rebased now. 🙂

[Qard]

I still seem to be getting yelled at by clippy for unrelated things. 🤔

The issues appear to be fixed by the clippy allows in #456, so maybe I'll just wait for that to land too? 😅

[Xenira]

Also needs a rebase and regeneration of the allowed_bindings.rs due to conflicts.

[ptondereau]

I think we also miss the opportunity to add an integration test by supporting setting ini_entries in SapiBuilder.

in https://github.com/davidcole1340/ext-php-rs/blob/060c8bfb2dacffe35cffb558d4f3738d86a5d34c/tests/sapi.rs

We could have in addition something like this:

#[test]
fn test_sapi() {
    let mut ini = IniBuilder::new();
    ini.define("foo=bar").unwrap();
    ini.quoted("memory_limit", "128M").unwrap();

    let mut builder = SapiBuilder::new("test", "Test");
    builder = builder.ini_entries(ini.finish());
// ...    

[Qard]

I've updated to include all the feedback except moving the CStringScope. Just need to figure out where to put that. 😅

[Xenira]

I've updated to include all the feedback except moving the CStringScope. Just need to figure out where to put that. 😅

While I am usually against util folders I guess its better than having it in the src root, as that is getting a little full by now. Can't think of anything better right now, so I'd put int into src/util.

Can always move it later if we can think of anything better :)

Other than that this looks really good. Thank you ❤️

[ptondereau]

LGTM thank you!

[Qard]

I moved that to src/util/cstring_scope.rs now. Anything else that needs doing, or is this good to land now? 🙂