Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for serializing hex strings without quotes #554

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aljen
Copy link

@aljen aljen commented Dec 11, 2024

Problem

Currently, RON supports parsing hex numbers in both quoted and unquoted form:

number: 0x1234    // works fine
number: 4660  // also works

However, when serializing such values, they are always wrapped in quotes:

pub fn as_hex_u16<S>(value: &u16, serializer: S) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    serializer.serialize_str(&format!("0x{:04X}", value))
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
pub struct AddressRange {
    #[serde(serialize_with = "as_hex_u16")]
    pub start: u16,
    #[serde(serialize_with = "as_hex_u16")]
    pub end: u16,
    pub length: u16,
}
start: "0x1234",
end: "0x1235"
length: 2

This creates an inconsistency between parsing and serialization capabilities, and makes the serialized output less elegant, especially in configuration files where hex numbers are commonly used (e.g., for colors, flags, or memory addresses).

Solution

This PR adds a new configuration option hex_as_raw (default: true) to PrettyConfig that allows hex strings to be serialized without quotes. When enabled:

// Input
let val = "0x1234";
let config = PrettyConfig::new().hex_as_raw(true);
to_string_pretty(&val, config)?;
// Output
number: 0x1234

Number is serialised clean and consistent with parsing capabilities.

The implementation includes:

  • New hex_as_raw configuration option
  • Helper function to validate hex string format
  • Comprehensive test suite covering various cases
  • Full backward compatibility (can be disabled via config)

Benefits

  1. Consistency: Serialization now matches RON's existing parsing capabilities
  2. Readability: Hex numbers without quotes are more readable and familiar
  3. Configurability: The feature can be enabled/disabled as needed
  4. Safety: Only valid hex strings (0x[0-9a-fA-F]+) are serialized without quotes

This enhancement makes RON's handling of hex numbers more elegant and consistent :)

  • I've included my change in CHANGELOG.md

src/ser/mod.rs Outdated
fn hex_as_raw(&self) -> bool {
self.pretty
.as_ref()
.map_or(true, |(ref config, _)| config.hex_as_raw)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that the hex_as_raw option is enabled when no pretty config is given. Same with the default in the PrettyConfig above. Usually pretty config options are opt-in, so I'd like to hear some justification for being opt-out.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good idea, I changed it so now it defaults to false

@juntyr
Copy link
Member

juntyr commented Dec 12, 2024

I didn't know we supported parsing integers from hex strings ... can you add a test for that?

@juntyr
Copy link
Member

juntyr commented Dec 12, 2024

I'm not sure if I'm in favor of this proposed change so far. If the goal is to be able to serialize all integers in binary, octal, or hex notation, I'd happily accept a proposal for that.

If you only want to serialize specific integers as hex, there is a (hacky) workaround that should work already. You could define a serde serialize_with function that first formats the number as a hex string, then constructs a RawValue from it, and then serializes that. It's not very efficient but it would already work in user code.

@juntyr
Copy link
Member

juntyr commented Dec 12, 2024

@aljen Thank you for submitting the PR! I've left some thoughts above :)

@aljen
Copy link
Author

aljen commented Jan 6, 2025

I didn't know we supported parsing integers from hex strings ... can you add a test for that?

You're right, it was late, I was thinking about parsing hex value as an integer, not a string - fixed in description, sorry for confusion =)

Sorry for long time to reply, I was on holiday vacations :)

@juntyr
Copy link
Member

juntyr commented Jan 6, 2025

No worries, happy New Year!

Do you want this PR to go in the suggested direction of serialising all integers as hex / octal / binary? It would require a small rewrite in this case.

If it's just about serialising a few values in hex, have you tried out the hack using raw values?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants