-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
script: Support sprintf-style substitutions in console methods #43897
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,8 @@ use js::rust::wrappers::{ | |
| JS_IdToValue, JS_Stringify, JS_ValueToSource, | ||
| }; | ||
| use js::rust::{ | ||
| CapturedJSStack, HandleObject, HandleValue, IdVector, ToString, describe_scripted_caller, | ||
| CapturedJSStack, HandleObject, HandleValue, IdVector, ToNumber, ToString, | ||
| describe_scripted_caller, | ||
| }; | ||
| use script_bindings::conversions::get_dom_class; | ||
|
|
||
|
|
@@ -82,19 +83,46 @@ impl Console { | |
| ) { | ||
| let cx = GlobalScope::get_cx(); | ||
|
|
||
| let arguments = messages | ||
| .iter() | ||
| .map(|msg| console_argument_from_handle_value(cx, *msg, &mut Vec::new())) | ||
| .collect(); | ||
| // If the first argument is a string, apply sprintf-style substitutions per the | ||
| // WHATWG Console spec formatter. The result is a single formatted string followed | ||
| // by any arguments that were not consumed by a substitution specifier. | ||
| let (arguments, embedder_msg) = if !messages.is_empty() && messages[0].is_string() { | ||
| let (formatted, consumed) = apply_sprintf_substitutions(cx, &messages); | ||
| let remaining = &messages[consumed..]; | ||
|
|
||
| let mut arguments: Vec<ConsoleArgument> = | ||
| vec![ConsoleArgument::String(formatted.clone())]; | ||
| for msg in remaining { | ||
| arguments.push(console_argument_from_handle_value( | ||
| cx, | ||
| *msg, | ||
| &mut Vec::new(), | ||
| )); | ||
| } | ||
|
|
||
| let embedder_msg = if remaining.is_empty() { | ||
| formatted | ||
| } else { | ||
| format!("{formatted} {}", stringify_handle_values(remaining)) | ||
| }; | ||
|
|
||
| (arguments, embedder_msg.into()) | ||
| } else { | ||
| let arguments = messages | ||
| .iter() | ||
| .map(|msg| console_argument_from_handle_value(cx, *msg, &mut Vec::new())) | ||
| .collect(); | ||
| (arguments, stringify_handle_values(&messages)) | ||
| }; | ||
|
|
||
| let stacktrace = (include_stacktrace == IncludeStackTrace::Yes) | ||
| .then_some(get_js_stack(*GlobalScope::get_cx())); | ||
| let console_message = Self::build_message(level.clone(), arguments, stacktrace); | ||
|
|
||
| Console::send_to_devtools(global, console_message); | ||
|
|
||
| let prefix = global.current_group_label().unwrap_or_default(); | ||
| let msgs = stringify_handle_values(&messages); | ||
| let formatted_message = format!("{prefix}{msgs}"); | ||
| let formatted_message = format!("{prefix}{embedder_msg}"); | ||
|
|
||
| Self::send_to_embedder(global, level, formatted_message); | ||
| } | ||
|
|
@@ -451,6 +479,116 @@ fn maybe_stringify_dom_object(cx: JSContext, value: HandleValue) -> Option<DOMSt | |
| Some(repr.into()) | ||
| } | ||
|
|
||
| /// Apply sprintf-style substitutions to console format arguments per the WHATWG Console spec. | ||
| /// | ||
| /// If the first argument is a string, it is treated as a format string where `%s`, `%d`, `%i`, | ||
| /// `%f`, `%o`, `%O`, and `%c` are replaced by subsequent arguments. Returns the formatted string | ||
| /// and the index of the first argument that was not consumed by a substitution. | ||
| /// | ||
| /// <https://console.spec.whatwg.org/#formatter> | ||
| #[expect(unsafe_code)] | ||
| fn apply_sprintf_substitutions(cx: JSContext, messages: &[HandleValue]) -> (String, usize) { | ||
| debug_assert!(!messages.is_empty() && messages[0].is_string()); | ||
|
|
||
| let js_string = ptr::NonNull::new(messages[0].to_string()).unwrap(); | ||
| let format_string = unsafe { jsstr_to_string(*cx, js_string) }; | ||
|
|
||
| let mut result = String::new(); | ||
| let mut arg_index = 1usize; | ||
| let mut chars = format_string.chars().peekable(); | ||
|
|
||
| while let Some(c) = chars.next() { | ||
| if c != '%' { | ||
| result.push(c); | ||
| continue; | ||
| } | ||
|
|
||
| match chars.peek().copied() { | ||
| Some('s') => { | ||
| chars.next(); | ||
| if arg_index < messages.len() { | ||
| result.push_str(&stringify_handle_value(messages[arg_index]).to_string()); | ||
| arg_index += 1; | ||
| } else { | ||
| result.push_str("%s"); | ||
| } | ||
| }, | ||
| Some('d') | Some('i') => { | ||
| let spec = chars.next().unwrap(); | ||
| if arg_index < messages.len() { | ||
| let num = unsafe { ToNumber(*cx, messages[arg_index]) }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this throw if the value is a BigInt/Symbol like https://tc39.es/ecma262/#sec-tonumber? If so we need some if checks to avoid that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay, on it |
||
| if num.is_err() { | ||
| unsafe { jsapi::JS_ClearPendingException(*cx) }; | ||
| } | ||
|
Comment on lines
+520
to
+522
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than clearing the exception I'd prefer if we didn't call
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, gecko has https://searchfox.org/firefox-main/rev/0989a082704f0bda8d370ccd57402645d834757e/dom/console/Console.cpp#1332 too so I guess it's fine.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay |
||
| arg_index += 1; | ||
| format_integer_substitution(&mut result, num); | ||
| } else { | ||
| result.push('%'); | ||
| result.push(spec); | ||
| } | ||
| }, | ||
| Some('f') => { | ||
| chars.next(); | ||
| if arg_index < messages.len() { | ||
| let num = unsafe { ToNumber(*cx, messages[arg_index]) }; | ||
| if num.is_err() { | ||
| unsafe { jsapi::JS_ClearPendingException(*cx) }; | ||
| } | ||
| arg_index += 1; | ||
| format_float_substitution(&mut result, num); | ||
| } else { | ||
| result.push_str("%f"); | ||
| } | ||
| }, | ||
| Some('o') | Some('O') => { | ||
| let spec = chars.next().unwrap(); | ||
| if arg_index < messages.len() { | ||
| result.push_str(&stringify_handle_value(messages[arg_index]).to_string()); | ||
| arg_index += 1; | ||
| } else { | ||
| result.push('%'); | ||
| result.push(spec); | ||
| } | ||
| }, | ||
| Some('c') => { | ||
| chars.next(); | ||
| if arg_index < messages.len() { | ||
| arg_index += 1; // consume but ignore CSS styling | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "css styling"? Why can we ignore the argument in this case?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In https://console.spec.whatwg.org/#formatter, the behaviour is not yet specified but the argument is used for styling console output in other browers and I don't think Servo supports styled console output at the moment.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I had naively assumed that |
||
| } | ||
| }, | ||
| Some('%') => { | ||
| chars.next(); | ||
| result.push('%'); | ||
| }, | ||
| _ => { | ||
| result.push('%'); | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| (result, arg_index) | ||
| } | ||
|
|
||
| fn format_integer_substitution(result: &mut String, num: Result<f64, ()>) { | ||
| match num { | ||
| Ok(n) if n.is_nan() => result.push_str("NaN"), | ||
| Ok(n) if n == f64::INFINITY => result.push_str("Infinity"), | ||
| Ok(n) if n == f64::NEG_INFINITY => result.push_str("-Infinity"), | ||
| Ok(n) => result.push_str(&(n.trunc() as i64).to_string()), | ||
| Err(_) => result.push_str("NaN"), | ||
| } | ||
| } | ||
|
|
||
| fn format_float_substitution(result: &mut String, num: Result<f64, ()>) { | ||
| match num { | ||
| Ok(n) if n.is_nan() => result.push_str("NaN"), | ||
| Ok(n) if n == f64::INFINITY => result.push_str("Infinity"), | ||
| Ok(n) if n == f64::NEG_INFINITY => result.push_str("-Infinity"), | ||
| Ok(n) => result.push_str(&n.to_string()), | ||
| Err(_) => result.push_str("NaN"), | ||
| } | ||
| } | ||
|
|
||
| fn stringify_handle_values(messages: &[HandleValue]) -> DOMString { | ||
| DOMString::from(itertools::join( | ||
| messages.iter().copied().map(stringify_handle_value), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.