Skip to content

Conversation

@edenw97
Copy link
Contributor

@edenw97 edenw97 commented Oct 23, 2025

No description provided.

@edenw97 edenw97 changed the title fix issue that interger 1000001 is written as 1000000 fix issue that integer 1000001 is written as 1000000 Oct 23, 2025
@emilio
Copy link
Member

emilio commented Oct 24, 2025

it seems we could just early-return for int values instead, right? That would be cleaner.

@edenw97
Copy link
Contributor Author

edenw97 commented Oct 24, 2025

it seems we could just early-return for int values instead, right? That would be cleaner.

I do try to return early, but it fails at case
["dimension", "-0", 0, "integer", "red"], " ",

where the -0 is printed as 0 with the '-" sign ignore.

@edenw97 edenw97 force-pushed the fix_interger_serialization branch from 85dc7b6 to 526ef82 Compare October 30, 2025 18:16
@edenw97
Copy link
Contributor Author

edenw97 commented Nov 13, 2025

@emilio Hi emilio, would you please review my comment and see if other feedback for the PR?

Copy link
Member

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I'll land the suggested clean-ups in a follow-up PR.


let notation = if value == 0.0 && value.is_sign_negative() {
// Negative zero. Work around #20596.
dest.write_str("-0")?;
Copy link
Member

Choose a reason for hiding this comment

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

let's return Ok(()); here

scientific: false,
}
} else if let Some(int_val) = int_value {
write!(dest, "{}", int_val)?;
Copy link
Member

Choose a reason for hiding this comment

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

Let's return Ok(()) here.

dtoa_short::write(dest, value)?
};

if int_value.is_none() && value.fract() == 0. && !notation.decimal_point && !notation.scientific
Copy link
Member

Choose a reason for hiding this comment

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

Then int_value.is_none() can be dropped

@emilio emilio enabled auto-merge November 29, 2025 11:34
@emilio emilio added this pull request to the merge queue Nov 29, 2025
Merged via the queue into servo:main with commit 452e6af Nov 29, 2025
15 checks passed
emilio added a commit that referenced this pull request Nov 29, 2025
@emilio
Copy link
Member

emilio commented Nov 29, 2025

#421 is the follow-up.

github-merge-queue bot pushed a commit that referenced this pull request Nov 29, 2025
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