Fix non-strings also being escaped when copied (#307)

* Fix non-strings also being escaped when copied

* Move copy escape logic into objdiff-core
This commit is contained in:
LagoLunatic
2025-12-23 14:31:36 -05:00
committed by GitHub
parent 67b28b7da1
commit 976e629f08
6 changed files with 75 additions and 49 deletions
+37 -13
View File
@@ -83,7 +83,7 @@ impl fmt::Display for DataType {
impl DataType {
pub fn display_labels(&self, endian: object::Endianness, bytes: &[u8]) -> Vec<String> {
let mut strs = Vec::new();
for (literal, label_override) in self.display_literals(endian, bytes) {
for (literal, label_override, _) in self.display_literals(endian, bytes) {
let label = label_override.unwrap_or_else(|| self.to_string());
strs.push(format!("{label}: {literal:?}"))
}
@@ -94,7 +94,7 @@ impl DataType {
&self,
endian: object::Endianness,
bytes: &[u8],
) -> Vec<(String, Option<String>)> {
) -> Vec<(String, Option<String>, Option<String>)> {
let mut strs = Vec::new();
if self.required_len().is_some_and(|l| bytes.len() < l) {
log::warn!(
@@ -118,34 +118,34 @@ impl DataType {
match self {
DataType::Int8 => {
let i = i8::from_ne_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));
if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int16 => {
let i = endian.read_i16_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));
if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int32 => {
let i = endian.read_i32_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));
if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Int64 => {
let i = endian.read_i64_bytes(bytes.try_into().unwrap());
strs.push((format!("{i:#x}"), None));
strs.push((format!("{i:#x}"), None, None));
if i < 0 {
strs.push((format!("{:#x}", ReallySigned(i)), None));
strs.push((format!("{:#x}", ReallySigned(i)), None, None));
}
}
DataType::Float => {
@@ -156,6 +156,7 @@ impl DataType {
object::Endianness::Big => f32::from_be_bytes(bytes),
}),
None,
None,
));
}
DataType::Double => {
@@ -166,10 +167,11 @@ impl DataType {
object::Endianness::Big => f64::from_be_bytes(bytes),
}),
None,
None,
));
}
DataType::Bytes => {
strs.push((format!("{bytes:#?}"), None));
strs.push((format!("{bytes:#?}"), None, None));
}
DataType::String => {
if let Some(nul_idx) = bytes.iter().position(|&c| c == b'\0') {
@@ -177,13 +179,17 @@ impl DataType {
// Special case to display (ASCII) as the label for ASCII-only strings.
let (cow, _, had_errors) = encoding_rs::UTF_8.decode(str_bytes);
if !had_errors && cow.is_ascii() {
strs.push((format!("{cow}"), Some("ASCII".into())));
let string = format!("{cow}");
let copy_string = escape_special_ascii_characters(string.clone());
strs.push((string, Some("ASCII".into()), Some(copy_string)));
}
for (encoding, encoding_name) in SUPPORTED_ENCODINGS {
let (cow, _, had_errors) = encoding.decode(str_bytes);
// Avoid showing ASCII-only strings more than once if the encoding is ASCII-compatible.
if !had_errors && (!encoding.is_ascii_compatible() || !cow.is_ascii()) {
strs.push((format!("{cow}"), Some(encoding_name.into())));
let string = format!("{cow}");
let copy_string = escape_special_ascii_characters(string.clone());
strs.push((string, Some(encoding_name.into()), Some(copy_string)));
}
}
}
@@ -499,3 +505,21 @@ pub struct RelocationOverride {
pub target: RelocationOverrideTarget,
pub addend: i64,
}
/// Escape ASCII characters such as \n or \t, but not Unicode characters such as \u{3000}.
/// Suitable for copying to clipboard.
fn escape_special_ascii_characters(value: String) -> String {
let mut escaped = String::new();
escaped.push('"');
for c in value.chars() {
if c.is_ascii() {
for e in c.escape_default() {
escaped.push(e);
}
} else {
escaped.push(c);
}
}
escaped.push('"');
escaped
}
+6 -2
View File
@@ -440,9 +440,13 @@ impl Arch for ArchPpc {
let simplified = ins.simplified().to_string();
let show_orig = orig != simplified;
let mut out = Vec::with_capacity(2);
out.push(ContextItem::Copy { value: simplified, label: None });
out.push(ContextItem::Copy { value: simplified, label: None, copy_string: None });
if show_orig {
out.push(ContextItem::Copy { value: orig, label: Some("original".to_string()) });
out.push(ContextItem::Copy {
value: orig,
label: Some("original".to_string()),
copy_string: None,
});
}
out
}
+25 -11
View File
@@ -366,7 +366,7 @@ impl From<&DiffText<'_>> for HighlightKind {
}
pub enum ContextItem {
Copy { value: String, label: Option<String> },
Copy { value: String, label: Option<String>, copy_string: Option<String> },
Navigate { label: String, symbol_index: usize, kind: SymbolNavigationKind },
Separator,
}
@@ -398,9 +398,9 @@ pub fn symbol_context(obj: &Object, symbol_index: usize) -> Vec<ContextItem> {
return Vec::new();
};
let mut out = Vec::new();
out.push(ContextItem::Copy { value: symbol.name.clone(), label: None });
out.push(ContextItem::Copy { value: symbol.name.clone(), label: None, copy_string: None });
if let Some(name) = &symbol.demangled_name {
out.push(ContextItem::Copy { value: name.clone(), label: None });
out.push(ContextItem::Copy { value: name.clone(), label: None, copy_string: None });
}
if symbol.section.is_some()
&& let Some(address) = symbol.virtual_address
@@ -408,6 +408,7 @@ pub fn symbol_context(obj: &Object, symbol_index: usize) -> Vec<ContextItem> {
out.push(ContextItem::Copy {
value: format!("{address:x}"),
label: Some("virtual address".to_string()),
copy_string: None,
});
}
out.append(&mut obj.arch.symbol_context(obj, symbol_index));
@@ -501,8 +502,8 @@ pub fn relocation_context(
let literals = display_ins_data_literals(obj, ins);
if !literals.is_empty() {
out.push(ContextItem::Separator);
for (literal, label_override) in literals {
out.push(ContextItem::Copy { value: literal, label: label_override });
for (literal, label_override, copy_string) in literals {
out.push(ContextItem::Copy { value: literal, label: label_override, copy_string });
}
}
}
@@ -598,24 +599,37 @@ pub fn instruction_context(
for byte in resolved.code {
hex_string.push_str(&format!("{byte:02x}"));
}
out.push(ContextItem::Copy { value: hex_string, label: Some("instruction bytes".to_string()) });
out.push(ContextItem::Copy {
value: hex_string,
label: Some("instruction bytes".to_string()),
copy_string: None,
});
out.append(&mut obj.arch.instruction_context(obj, resolved));
if let Some(virtual_address) = resolved.symbol.virtual_address {
let offset = resolved.ins_ref.address - resolved.symbol.address;
out.push(ContextItem::Copy {
value: format!("{:x}", virtual_address + offset),
label: Some("virtual address".to_string()),
copy_string: None,
});
}
for arg in &ins.args {
if let InstructionArg::Value(arg) = arg {
out.push(ContextItem::Copy { value: arg.to_string(), label: None });
out.push(ContextItem::Copy { value: arg.to_string(), label: None, copy_string: None });
match arg {
InstructionArgValue::Signed(v) => {
out.push(ContextItem::Copy { value: v.to_string(), label: None });
out.push(ContextItem::Copy {
value: v.to_string(),
label: None,
copy_string: None,
});
}
InstructionArgValue::Unsigned(v) => {
out.push(ContextItem::Copy { value: v.to_string(), label: None });
out.push(ContextItem::Copy {
value: v.to_string(),
label: None,
copy_string: None,
});
}
_ => {}
}
@@ -677,7 +691,7 @@ pub fn instruction_hover(
let literals = display_ins_data_literals(obj, resolved);
if !literals.is_empty() {
out.push(HoverItem::Separator);
for (literal, label_override) in literals {
for (literal, label_override, _) in literals {
out.push(HoverItem::Text {
label: label_override.unwrap_or_else(|| ty.to_string()),
value: format!("{literal:?}"),
@@ -871,7 +885,7 @@ pub fn display_ins_data_labels(obj: &Object, resolved: ResolvedInstructionRef) -
pub fn display_ins_data_literals(
obj: &Object,
resolved: ResolvedInstructionRef,
) -> Vec<(String, Option<String>)> {
) -> Vec<(String, Option<String>, Option<String>)> {
let Some(reloc) = resolved.relocation else {
return Vec::new();
};
+2 -21
View File
@@ -865,24 +865,6 @@ pub fn hover_items_ui(ui: &mut Ui, items: Vec<HoverItem>, appearance: &Appearanc
}
}
/// Escape ASCII characters such as \n or \t, but not Unicode characters such as \u{3000}.
/// Suitable for copying to clipboard.
fn escape_special_ascii_characters(value: String) -> String {
let mut escaped = String::new();
escaped.push('"');
for c in value.chars() {
if c.is_ascii() {
for e in c.escape_default() {
escaped.push(e);
}
} else {
escaped.push(c);
}
}
escaped.push('"');
escaped
}
pub fn context_menu_items_ui(
ui: &mut Ui,
items: Vec<ContextItem>,
@@ -892,7 +874,7 @@ pub fn context_menu_items_ui(
let mut ret = None;
for item in items {
match item {
ContextItem::Copy { value, label } => {
ContextItem::Copy { value, label, copy_string } => {
let mut job = LayoutJob::default();
write_text("Copy ", appearance.text_color, &mut job, appearance.code_font.clone());
write_text(
@@ -912,8 +894,7 @@ pub fn context_menu_items_ui(
write_text(")", appearance.text_color, &mut job, appearance.code_font.clone());
}
if ui.button(job).clicked() {
let escaped = escape_special_ascii_characters(value);
ui.ctx().copy_text(escaped);
ui.ctx().copy_text(copy_string.unwrap_or(value));
ui.close();
}
}
+4 -2
View File
@@ -273,6 +273,7 @@ impl GuestDisplay for Component {
return vec![ContextItem::Copy(ContextItemCopy {
value: "Failed to resolve instruction".to_string(),
label: Some("error".to_string()),
copy_string: None,
})];
};
let ins = match obj.arch.process_instruction(resolved, &diff_config) {
@@ -281,6 +282,7 @@ impl GuestDisplay for Component {
return vec![ContextItem::Copy(ContextItemCopy {
value: e.to_string(),
label: Some("error".to_string()),
copy_string: None,
})];
}
};
@@ -707,8 +709,8 @@ impl From<diff::display::HoverItemColor> for HoverItemColor {
impl From<diff::display::ContextItem> for ContextItem {
fn from(item: diff::display::ContextItem) -> Self {
match item {
diff::display::ContextItem::Copy { value, label } => {
ContextItem::Copy(ContextItemCopy { value, label })
diff::display::ContextItem::Copy { value, label, copy_string } => {
ContextItem::Copy(ContextItemCopy { value, label, copy_string })
}
diff::display::ContextItem::Navigate { label, symbol_index, kind } => {
ContextItem::Navigate(ContextItemNavigate {
+1
View File
@@ -143,6 +143,7 @@ interface display {
record context-item-copy {
value: string,
label: option<string>,
copy-string: option<string>,
}
record context-item-navigate {