feat: Add Dhall support #466

polarathene · 2023-10-04T23:13:09Z

Rebased #218 with conflicts resolved. All credit to @sphw for the actual contribution.

This PR adds Dhall support using serde_dhall, a native Rust implementation of Dhall. Dhall is a functional configuration language designed to make composition of configuration files easier.

Closes #123

polarathene · 2023-10-05T20:20:51Z

@matthiasbeyer since this original PR was stale from about 2 years ago, does anything look like adjustments are needed? In the tests I can see it's missing the enum settings file like all other config types seem to have coverage for. Should that be addressed?

I probably should version bump the new dependencies added too?

polarathene · 2023-10-05T20:50:18Z

src/file/format/dhall.rs

+ serde_dhall::SimpleValue::Num(num) => match num {
+ serde_dhall::NumKind::Bool(b) => Value::new(uri, ValueKind::Boolean(b)),
+ serde_dhall::NumKind::Natural(n) => Value::new(uri, ValueKind::Integer(n as i64)),
+ serde_dhall::NumKind::Integer(i) => Value::new(uri, ValueKind::Integer(i)),


TOML seems to rely on inference while others are more explicitly typing to ValueKind::I64(value), should I go with the latter?

config-rs/src/file/format/toml.rs

Line 24 in d7c1656

toml::Value::Integer(value) => Value::new(uri, value),

config-rs/src/file/format/ron.rs

Line 34 in d7c1656

ron::Number::Integer(value) => ValueKind::I64(value),

config-rs/src/file/format/json.rs

Lines 24 to 26 in d7c1656

serde_json::Value::Number(ref value) => {

if let Some(value) = value.as_i64() {

Value::new(uri, ValueKind::I64(value))

config-rs/src/file/format/yaml.rs

Line 49 in d7c1656

yaml::Yaml::Integer(value) => Ok(Value::new(uri, ValueKind::I64(value))),

polarathene

While looking into version bumping serde_dhall I checked the repo and noticed it's no longer maintained.

The demand for Dhall in config-rs seems low. Perhaps this isn't worth landing into config-rs?

polarathene · 2023-10-05T20:58:54Z

Cargo.toml

@@ -36,6 +37,7 @@ yaml-rust = { version = "0.4", optional = true }
 rust-ini = { version = "0.19", optional = true }
 ron = { version = "0.8", optional = true }
 json5_rs = { version = "0.4", optional = true, package = "json5" }
+serde_dhall = { version = "0.10", optional = true }


Suggested change

serde_dhall = { version = "0.10", optional = true }

serde_dhall = { version = "0.12", optional = true }

Note this status update from June 2023:

https://github.com/Nadrieril/dhall-rust#status

I am no longer maintaining this project.
I got it to support about 90% of the language but then lost faith in the usability of dhall for my purposes

Only 1 other user has 👍 the original issue (Nov 2019) requesting Dhall support: Format request: Dhall #123

The original PR (July 2021) itself was left stale for about 2 years, with the author no longer interested pursuing it.

The original issue author also expressed a loss of interest in Dhall (March 2021).

I personally have no interest in the config format myself. I just thought I'd help avoid a feature PR from going stale 😅

Given the above, perhaps Dhall should not be merged into config-rs? The lack of upstream maintenance and demand doesn't seem to justify any additional maintenance burden it may offload onto config-rs project?

src/file/format/dhall.rs

+ let value = from_dhall_value(uri, serde_dhall::from_str(text).parse()?);
+ match value.kind {
+ ValueKind::Table(map) => Ok(map),
+ ValueKind::Nil => Err(Unexpected::Unit),
+ ValueKind::Boolean(value) => Err(Unexpected::Bool(value)),
+ ValueKind::Integer(value) => Err(Unexpected::Integer(value)),
+ ValueKind::Float(value) => Err(Unexpected::Float(value)),
+ ValueKind::String(value) => Err(Unexpected::Str(value)),
+ ValueKind::Array(value) => Err(Unexpected::Seq),
+ }
+ .map_err(|err| ConfigError::invalid_root(uri, err))
+ .map_err(|err| Box::new(err) as Box<dyn Error + Send + Sync>)


Signed-off-by: Brennan Kinney <[email protected]>

- fix: `HashMap` => `Map` to avoid compilation error Original author: MGlolenstine - fix: `Integer` => `I64` `ValueKind` enum has changed since the original PR went stale. - chore: Version bump `serde_dhall` to `0.12` Signed-off-by: Brennan Kinney <[email protected]>

matthiasbeyer · 2023-10-06T06:42:18Z

The demand for Dhall in config-rs seems low. Perhaps this isn't worth landing into config-rs?

Yes, depending on an unmaintained implementation is definitively something we shouldn't rely on, sorry.

polarathene · 2023-10-07T09:33:50Z

FWIW, it seems that the yaml support has become unmaintained, no commits since July 2021. The developer doesn't have much activity on Github (last active Sep 2022).

polarathene mentioned this pull request Oct 4, 2023

Add Dhall support #218

Closed

matthiasbeyer approved these changes Oct 5, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

polarathene commented Oct 5, 2023

View reviewed changes

sphw and others added 2 commits October 6, 2023 11:23

feat: Add Dhall support

fe1b043

Signed-off-by: Brennan Kinney <[email protected]>

polarathene force-pushed the add-dhall-support branch from 7a4bcaa to 35a8b56 Compare October 5, 2023 22:28

polarathene mentioned this pull request Oct 6, 2023

chore: Use a common method in parsers to check root is a table #469

Merged

matthiasbeyer mentioned this pull request Oct 7, 2023

Remove YAML support #473

Closed

polarathene mentioned this pull request Oct 19, 2023

refactor: Use a common from_value method #472

Open

matthiasbeyer added the hacktoberfest-accepted Accepted PR for hacktoberfest label Oct 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Dhall support #466

feat: Add Dhall support #466

polarathene commented Oct 4, 2023

polarathene commented Oct 5, 2023 •

edited

This comment was marked as outdated.

polarathene Oct 5, 2023

polarathene left a comment

polarathene Oct 5, 2023 •

edited

This comment was marked as resolved.

matthiasbeyer commented Oct 6, 2023

polarathene commented Oct 7, 2023

	serde_json::Value::Number(ref value) => {
	if let Some(value) = value.as_i64() {
	Value::new(uri, ValueKind::I64(value))

	serde_dhall = { version = "0.10", optional = true }
	serde_dhall = { version = "0.12", optional = true }

feat: Add Dhall support #466

Are you sure you want to change the base?

feat: Add Dhall support #466

Conversation

polarathene commented Oct 4, 2023

polarathene commented Oct 5, 2023 • edited

This comment was marked as outdated.

polarathene Oct 5, 2023

Choose a reason for hiding this comment

polarathene left a comment

Choose a reason for hiding this comment

polarathene Oct 5, 2023 • edited

Choose a reason for hiding this comment

This comment was marked as resolved.

matthiasbeyer commented Oct 6, 2023

polarathene commented Oct 7, 2023

polarathene commented Oct 5, 2023 •

edited

polarathene Oct 5, 2023 •

edited