Skip to content

Commit afd8680

Browse files
authored
fix: console.log formatting (#8702)
* fix: console.log formatting * no deref * clippy
1 parent 1710187 commit afd8680

File tree

1 file changed

+58
-52
lines changed

1 file changed

+58
-52
lines changed

crates/common/fmt/src/console.rs

Lines changed: 58 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,17 @@ pub enum FormatSpec {
2121
}
2222

2323
impl FormatSpec {
24-
fn from_chars<I>(iter: &mut Peekable<I>) -> Option<Self>
24+
fn from_chars<I>(iter: &mut Peekable<I>) -> Result<Self, String>
2525
where
2626
I: Iterator<Item = char>,
2727
{
28-
match iter.next()? {
29-
's' => Some(Self::String),
30-
'd' => Some(Self::Number),
31-
'i' => Some(Self::Integer),
32-
'o' => Some(Self::Object),
33-
'e' => Some(Self::Exponential(None)),
34-
'x' => Some(Self::Hexadecimal),
28+
match iter.next().ok_or_else(String::new)? {
29+
's' => Ok(Self::String),
30+
'd' => Ok(Self::Number),
31+
'i' => Ok(Self::Integer),
32+
'o' => Ok(Self::Object),
33+
'e' => Ok(Self::Exponential(None)),
34+
'x' => Ok(Self::Hexadecimal),
3535
ch if ch.is_ascii_digit() => {
3636
let mut num = ch.to_string();
3737
while let Some(&ch) = iter.peek() {
@@ -44,16 +44,17 @@ impl FormatSpec {
4444
}
4545
if let Some(&ch) = iter.peek() {
4646
if ch == 'e' {
47+
let num = num.parse().map_err(|_| num)?;
4748
iter.next();
48-
Some(Self::Exponential(Some(num.parse().ok()?)))
49+
Ok(Self::Exponential(Some(num)))
4950
} else {
50-
None
51+
Err(num)
5152
}
5253
} else {
53-
None
54+
Err(num)
5455
}
5556
}
56-
_ => None,
57+
ch => Err(String::from(ch)),
5758
}
5859
}
5960
}
@@ -248,76 +249,74 @@ impl ConsoleFmt for [u8] {
248249
/// assert_eq!(formatted, "foo has 3 characters");
249250
/// ```
250251
pub fn console_format(spec: &str, values: &[&dyn ConsoleFmt]) -> String {
251-
let mut values = values.iter().copied();
252+
let mut values = values.iter().copied().peekable();
252253
let mut result = String::with_capacity(spec.len());
253254

254255
// for the first space
255-
let mut write_space = true;
256-
let last_value = if spec.is_empty() {
257-
// we still want to print any remaining values
258-
write_space = false;
259-
values.next()
256+
let mut write_space = if spec.is_empty() {
257+
false
260258
} else {
261-
format_spec(spec, &mut values, &mut result)
259+
format_spec(spec, &mut values, &mut result);
260+
true
262261
};
263262

264263
// append any remaining values with the standard format
265-
if let Some(v) = last_value {
266-
for v in std::iter::once(v).chain(values) {
267-
let fmt = v.fmt(FormatSpec::String);
268-
if write_space {
269-
result.push(' ');
270-
}
271-
result.push_str(&fmt);
272-
write_space = true;
264+
for v in values {
265+
let fmt = v.fmt(FormatSpec::String);
266+
if write_space {
267+
result.push(' ');
273268
}
269+
result.push_str(&fmt);
270+
write_space = true;
274271
}
275272

276273
result
277274
}
278275

279276
fn format_spec<'a>(
280277
s: &str,
281-
values: &mut impl Iterator<Item = &'a dyn ConsoleFmt>,
278+
values: &mut Peekable<impl Iterator<Item = &'a dyn ConsoleFmt>>,
282279
result: &mut String,
283-
) -> Option<&'a dyn ConsoleFmt> {
280+
) {
284281
let mut expect_fmt = false;
285-
let mut current_value = values.next();
286282
let mut chars = s.chars().peekable();
287283

288-
while let Some(ch) = chars.next() {
284+
while chars.peek().is_some() {
289285
if expect_fmt {
290286
expect_fmt = false;
291-
let mut iter = std::iter::once(ch).chain(chars.by_ref()).peekable();
292-
if let Some(spec) = FormatSpec::from_chars(&mut iter) {
293-
// format and write the value
294-
if let Some(value) = current_value {
295-
let string = value.fmt(spec);
296-
result.push_str(&string);
297-
current_value = values.next();
287+
match FormatSpec::from_chars(&mut chars) {
288+
Ok(spec) => {
289+
let value = values.next().expect("value existence is checked");
290+
// format and write the value
291+
result.push_str(&value.fmt(spec));
298292
}
299-
} else {
300-
// invalid specifier or a second `%`, in both cases we ignore
301-
result.push('%');
302-
result.push(ch);
303-
}
304-
} else if ch == '%' {
305-
if let Some(&next_ch) = chars.peek() {
306-
if next_ch == '%' {
293+
Err(consumed) => {
294+
// on parser failure, write '%' and consumed characters
307295
result.push('%');
308-
chars.next();
296+
result.push_str(&consumed);
297+
}
298+
}
299+
} else {
300+
let ch = chars.next().unwrap();
301+
if ch == '%' {
302+
if let Some(&next_ch) = chars.peek() {
303+
if next_ch == '%' {
304+
result.push('%');
305+
chars.next();
306+
} else if values.peek().is_some() {
307+
// only try formatting if there are values to format
308+
expect_fmt = true;
309+
} else {
310+
result.push(ch);
311+
}
309312
} else {
310-
expect_fmt = true;
313+
result.push(ch);
311314
}
312315
} else {
313316
result.push(ch);
314317
}
315-
} else {
316-
result.push(ch);
317318
}
318319
}
319-
320-
current_value
321320
}
322321

323322
#[cfg(test)]
@@ -468,6 +467,13 @@ mod tests {
468467
fmt_1("%x", &I256::try_from(-100000000000i64).unwrap())
469468
);
470469
assert_eq!("100", fmt_1("%o", &I256::try_from(100).unwrap()));
470+
471+
// make sure that %byte values are not consumed when there are no values
472+
assert_eq!("%333d%3e%5F", console_format("%333d%3e%5F", &[]));
473+
assert_eq!(
474+
"%5d123456.789%2f%3f%e1",
475+
console_format("%5d%3e%2f%3f%e1", &[&U256::from(123456789)])
476+
);
471477
}
472478

473479
#[test]

0 commit comments

Comments
 (0)