objdiff-cli diff: Click-to-highlight & build fixes

objdiff-cli/src/cmd/diff.rs

@@ -9,7 +9,8 @@ use crossterm::{
     cursor::{Hide, MoveRight, MoveTo, Show},
     event,
     event::{
-        DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEventKind, MouseEventKind,
+        DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEventKind, MouseButton,
+        MouseEventKind,
     },
     style::{Color, PrintStyledContent, Stylize},
     terminal::{
@@ -22,7 +23,7 @@ use objdiff_core::{
     diff,
     diff::display::{display_diff, DiffText},
     obj,
-    obj::{ObjInfo, ObjInsDiffKind, ObjSection, ObjSectionKind, ObjSymbol},
+    obj::{ObjInfo, ObjInsArgValue, ObjInsDiffKind, ObjSection, ObjSectionKind, ObjSymbol},
 };
 
 use crate::util::term::crossterm_panic_handler;
@@ -69,17 +70,22 @@ pub fn run(args: Args) -> Result<()> {
         EnableMouseCapture,
     )?;
 
+    let mut clear = true;
     let mut redraw = true;
     let mut skip = 0;
+    let mut click_xy = None;
+    let mut highlight = HighlightKind::None;
+    let (mut sx, mut sy) = terminal_size()?;
     loop {
         let y_offset = 2;
-        let (sx, sy) = terminal_size()?;
         let per_page = sy as usize - y_offset;
         if redraw {
             let mut w = stdout().lock();
+            if clear {
+                crossterm::queue!(w, Clear(ClearType::All))?;
+            }
             crossterm::queue!(
                 w,
-                Clear(ClearType::All),
                 MoveTo(0, 0),
                 PrintStyledContent(args.symbol.clone().with(Color::White)),
                 MoveTo(0, 1),
@@ -103,14 +109,51 @@ pub fn run(args: Args) -> Result<()> {
             if skip > max_len - per_page {
                 skip = max_len - per_page;
             }
+            let mut new_highlight = None;
             if let Some((_, symbol)) = left_sym {
-                print_sym(&mut w, symbol, 0, y_offset as u16, sx / 2 - 1, sy, skip)?;
+                let h = print_sym(
+                    &mut w,
+                    symbol,
+                    0,
+                    y_offset as u16,
+                    sx / 2 - 1,
+                    sy,
+                    skip,
+                    &mut highlight,
+                    click_xy,
+                )?;
+                if let Some(h) = h {
+                    new_highlight = Some(h);
+                }
             }
             if let Some((_, symbol)) = right_sym {
-                print_sym(&mut w, symbol, sx / 2, y_offset as u16, sx, sy, skip)?;
+                let h = print_sym(
+                    &mut w,
+                    symbol,
+                    sx / 2,
+                    y_offset as u16,
+                    sx,
+                    sy,
+                    skip,
+                    &mut highlight,
+                    click_xy,
+                )?;
+                if let Some(h) = h {
+                    new_highlight = Some(h);
+                }
             }
             w.flush()?;
-            redraw = false;
+            if let Some(new_highlight) = new_highlight {
+                highlight = new_highlight;
+                redraw = true;
+                click_xy = None;
+                clear = false;
+                continue; // Redraw now
+            } else {
+                redraw = false;
+                click_xy = None;
+                clear = true;
+            }
         }
 
         match event::read()? {
@@ -167,9 +210,17 @@ pub fn run(args: Args) -> Result<()> {
                     skip = skip.saturating_sub(3);
                     redraw = true;
                 }
+                MouseEventKind::Down(MouseButton::Left) => {
+                    click_xy = Some((event.column, event.row));
+                    redraw = true;
+                }
                 _ => {}
             },
-            Event::Resize(_, _) => redraw = true,
+            Event::Resize(x, y) => {
+                sx = x;
+                sy = y;
+                redraw = true;
+            }
             _ => {}
         }
     }
@@ -194,6 +245,7 @@ fn find_function<'a>(obj: &'a ObjInfo, name: &str) -> Option<(&'a ObjSection, &'
     None
 }
 
+#[allow(clippy::too_many_arguments)]
 fn print_sym<W>(
     w: &mut W,
     symbol: &ObjSymbol,
@@ -202,11 +254,14 @@ fn print_sym<W>(
     max_sx: u16,
     max_sy: u16,
     skip: usize,
-) -> Result<()>
+    highlight: &mut HighlightKind,
+    click_xy: Option<(u16, u16)>,
+) -> Result<Option<HighlightKind>>
 where
     W: Write,
 {
     let base_addr = symbol.address as u32;
+    let mut new_highlight = None;
     for ins_diff in symbol.instructions.iter().skip(skip) {
         let mut sx = sx;
         if ins_diff.kind != ObjInsDiffKind::None && sx > 2 {
@@ -220,7 +275,7 @@ where
         } else {
             crossterm::queue!(w, MoveTo(sx, sy))?;
         }
-        display_diff(ins_diff, base_addr, |text| {
+        display_diff(ins_diff, base_addr, |text| -> Result<()> {
             let mut label_text;
             let mut base_color = match ins_diff.kind {
                 ObjInsDiffKind::None | ObjInsDiffKind::OpMismatch | ObjInsDiffKind::ArgMismatch => {
@@ -231,6 +286,7 @@ where
                 ObjInsDiffKind::Insert => Color::DarkGreen,
             };
             let mut pad_to = 0;
+            let mut highlight_kind = HighlightKind::None;
             match text {
                 DiffText::Basic(text) => {
                     label_text = text.to_string();
@@ -247,27 +303,32 @@ where
                 DiffText::Address(addr) => {
                     label_text = format!("{:x}:", addr);
                     pad_to = 5;
+                    highlight_kind = HighlightKind::Address(addr);
                 }
-                DiffText::Opcode(mnemonic, _op) => {
+                DiffText::Opcode(mnemonic, op) => {
                     label_text = mnemonic.to_string();
                     if ins_diff.kind == ObjInsDiffKind::OpMismatch {
                         base_color = Color::Blue;
                     }
                     pad_to = 8;
+                    highlight_kind = HighlightKind::Opcode(op);
                 }
                 DiffText::Argument(arg, diff) => {
                     label_text = arg.to_string();
                     if let Some(diff) = diff {
                         base_color = COLOR_ROTATION[diff.idx % COLOR_ROTATION.len()]
                     }
+                    highlight_kind = HighlightKind::Arg(arg.clone());
                 }
                 DiffText::BranchTarget(addr) => {
                     label_text = format!("{addr:x}");
+                    highlight_kind = HighlightKind::Address(addr);
                 }
                 DiffText::Symbol(sym) => {
                     let name = sym.demangled_name.as_ref().unwrap_or(&sym.name);
                     label_text = name.clone();
                     base_color = Color::White;
+                    highlight_kind = HighlightKind::Symbol(name.clone());
                 }
                 DiffText::Spacing(n) => {
                     crossterm::queue!(w, MoveRight(n as u16))?;
@@ -283,8 +344,22 @@ where
             if sx >= max_sx {
                 return Ok(());
             }
+            let highlighted = highlight == &highlight_kind;
+            if let Some((cx, cy)) = click_xy {
+                if cx >= sx && cx < sx + len as u16 && cy == sy {
+                    if highlighted {
+                        new_highlight = Some(HighlightKind::None);
+                    } else {
+                        new_highlight = Some(highlight_kind);
+                    }
+                }
+            }
             label_text.truncate(max_sx as usize - sx as usize);
-            crossterm::queue!(w, PrintStyledContent(label_text.with(base_color)))?;
+            let mut content = label_text.with(base_color);
+            if highlighted {
+                content = content.on_dark_grey();
+            }
+            crossterm::queue!(w, PrintStyledContent(content))?;
             sx += len as u16;
             if pad_to > len {
                 let pad = (pad_to - len) as u16;
@@ -297,7 +372,7 @@ where
             break;
         }
     }
-    Ok(())
+    Ok(new_highlight)
 }
 
 pub const COLOR_ROTATION: [Color; 8] = [
@@ -320,3 +395,26 @@ pub fn match_percent_color(match_percent: f32) -> Color {
         Color::Red
     }
 }
+
+#[derive(Default)]
+pub enum HighlightKind {
+    #[default]
+    None,
+    Opcode(u8),
+    Arg(ObjInsArgValue),
+    Symbol(String),
+    Address(u32),
+}
+
+impl PartialEq for HighlightKind {
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (HighlightKind::None, HighlightKind::None) => false,
+            (HighlightKind::Opcode(a), HighlightKind::Opcode(b)) => a == b,
+            (HighlightKind::Arg(a), HighlightKind::Arg(b)) => a.loose_eq(b),
+            (HighlightKind::Symbol(a), HighlightKind::Symbol(b)) => a == b,
+            (HighlightKind::Address(a), HighlightKind::Address(b)) => a == b,
+            _ => false,
+        }
+    }
+}

objdiff-cli/src/cmd/report.rs

@@ -178,7 +178,7 @@ fn report_object(
         .as_ref()
         .map(|p| obj::elf::read(p).with_context(|| format!("Failed to open {}", p.display())))
         .transpose()?;
-    let config = diff::DiffObjConfig { relax_reloc_diffs: true, ..Default::default() };
+    let config = diff::DiffObjConfig { relax_reloc_diffs: true };
     diff::diff_objs(&config, target.as_mut(), base.as_mut())?;
     let mut unit = ReportUnit { name: object.name().to_string(), ..Default::default() };
     let obj = target.as_ref().or(base.as_ref()).unwrap();

objdiff-core/src/diff/display.rs

@@ -1,7 +1,5 @@
 use std::cmp::Ordering;
 
-use anyhow::{bail, Result};
-
 use crate::obj::{
     ObjInsArg, ObjInsArgDiff, ObjInsArgValue, ObjInsDiff, ObjReloc, ObjRelocKind, ObjSymbol,
 };
@@ -30,11 +28,11 @@ pub enum DiffText<'a> {
     Eol,
 }
 
-pub fn display_diff(
+pub fn display_diff<E>(
     ins_diff: &ObjInsDiff,
     base_addr: u32,
-    mut cb: impl FnMut(DiffText) -> Result<()>,
-) -> Result<()> {
+    mut cb: impl FnMut(DiffText) -> Result<(), E>,
+) -> Result<(), E> {
     let Some(ins) = &ins_diff.ins else {
         cb(DiffText::Eol)?;
         return Ok(());
@@ -94,7 +92,10 @@ pub fn display_diff(
     Ok(())
 }
 
-fn display_reloc_name(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) -> Result<()> {
+fn display_reloc_name<E>(
+    reloc: &ObjReloc,
+    mut cb: impl FnMut(DiffText) -> Result<(), E>,
+) -> Result<(), E> {
     cb(DiffText::Symbol(&reloc.target))?;
     match reloc.target.addend.cmp(&0i64) {
         Ordering::Greater => cb(DiffText::Basic(&format!("+{:#X}", reloc.target.addend))),
@@ -103,7 +104,10 @@ fn display_reloc_name(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<(
     }
 }
 
-fn display_reloc(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) -> Result<()> {
+fn display_reloc<E>(
+    reloc: &ObjReloc,
+    mut cb: impl FnMut(DiffText) -> Result<(), E>,
+) -> Result<(), E> {
     match reloc.kind {
         #[cfg(feature = "ppc")]
         ObjRelocKind::PpcAddr16Lo => {
@@ -165,7 +169,7 @@ fn display_reloc(reloc: &ObjReloc, mut cb: impl FnMut(DiffText) -> Result<()>) -
         }
         #[cfg(feature = "mips")]
         ObjRelocKind::MipsGpRel32 => {
-            bail!("unimplemented: mips gp_rel32");
+            todo!("unimplemented: mips gp_rel32");
         }
         ObjRelocKind::Absolute => {
             cb(DiffText::Basic("[INVALID]"))?;

objdiff-gui/src/app.rs

@@ -12,11 +12,8 @@ use std::{
 use filetime::FileTime;
 use globset::{Glob, GlobSet};
 use notify::{RecursiveMode, Watcher};
-use objdiff_core::{
-    config::{
-        build_globset, ProjectConfigInfo, ProjectObject, ScratchConfig, DEFAULT_WATCH_PATTERNS,
-    },
-    diff::DiffAlg,
+use objdiff_core::config::{
+    build_globset, ProjectConfigInfo, ProjectObject, ScratchConfig, DEFAULT_WATCH_PATTERNS,
 };
 use time::UtcOffset;
 
@@ -29,9 +26,7 @@ use crate::{
     },
     views::{
         appearance::{appearance_window, Appearance},
-        config::{
-            config_ui, diff_options_window, project_window, ConfigViewState, CONFIG_DISABLED_TEXT,
-        },
+        config::{config_ui, project_window, ConfigViewState, CONFIG_DISABLED_TEXT},
         data_diff::data_diff_ui,
         debug::debug_window,
         demangle::{demangle_window, DemangleViewState},
@@ -529,7 +524,6 @@ impl eframe::App for App {
         project_window(ctx, config, show_project_config, config_state, appearance);
         appearance_window(ctx, show_appearance_config, appearance);
         demangle_window(ctx, show_demangle, demangle_state, appearance);
-        diff_options_window(ctx, config, show_diff_options, appearance);
         debug_window(ctx, show_debug, frame_history, appearance);
 
         self.post_update(ctx);

objdiff-gui/src/jobs/objdiff.rs

@@ -7,7 +7,7 @@ use std::{
 
 use anyhow::{anyhow, Context, Error, Result};
 use objdiff_core::{
-    diff::{diff_objs, DiffAlg, DiffObjConfig},
+    diff::{diff_objs, DiffObjConfig},
     obj::{elf, ObjInfo},
 };
 use time::OffsetDateTime;

objdiff-gui/src/views/config.rs

@@ -11,7 +11,7 @@ use anyhow::{Context, Result};
 use const_format::formatcp;
 use egui::{
     output::OpenUrl, text::LayoutJob, CollapsingHeader, FontFamily, FontId, RichText,
-    SelectableLabel, TextFormat, Widget, WidgetText,
+    SelectableLabel, TextFormat, Widget,
 };
 use globset::Glob;
 use objdiff_core::config::{ProjectObject, DEFAULT_WATCH_PATTERNS};
@@ -838,15 +838,3 @@ fn split_obj_config_ui(
         }
     });
 }
-
-pub fn diff_options_window(
-    ctx: &egui::Context,
-    config: &AppConfigRef,
-    show: &mut bool,
-    appearance: &Appearance,
-) {
-    let mut config_guard = config.write().unwrap();
-    egui::Window::new("Diff Options").open(show).show(ctx, |ui| {
-        diff_options_ui(ui, &mut config_guard, appearance);
-    });
-}

objdiff-gui/src/views/function_diff.rs

@@ -247,7 +247,7 @@ fn asm_row_ui(
     let space_width = ui.fonts(|f| f.glyph_width(&appearance.code_font, ' '));
     display_diff(ins_diff, symbol.address as u32, |text| {
         diff_text_ui(ui, text, ins_diff, appearance, ins_view_state, space_width);
-        Ok(())
+        Ok::<_, ()>(())
     })
     .unwrap();
 }