Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,10 @@ Thumbs.db
tmp/
.temp/
dist/

# Local agent/tooling installs
/.agents/
/.claude/
/node_modules/
/package.json
/bun.lock
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reconsider ignoring bun.lock.

Lock files like bun.lock ensure that all developers and CI/CD pipelines use identical dependency versions, which is critical for reproducible builds. Ignoring it can lead to "works on my machine" issues and inconsistent behavior across environments.

Best practice is to commit lock files to version control. If there's a specific reason to ignore it (e.g., it's generated by a local development tool and not part of the build process), please document that reasoning.

🔧 Proposed fix
 /.agents/
 /.claude/
 /node_modules/
-/package.json
-/bun.lock
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.gitignore at line 42, Remove the /bun.lock entry from .gitignore (or, if
there is a deliberate reason to exclude it, add a short comment above that line
explaining why it must remain ignored) so that the bun.lock lockfile is
committed for reproducible builds; reference the existing /bun.lock ignore entry
to locate and update the file.

7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ serde_json = "1"
image = { version = "0.25", default-features = false, features = ["png"] }
chrono = { version = "0.4", default-features = false, features = ["clock"] }
clap = { version = "4.5", features = ["derive"] }
base64 = "0.22"

# GTK stack
gtk4 = { version = "0.8", package = "gtk4" }
Expand Down
8 changes: 7 additions & 1 deletion src/app/tray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,13 @@ pub fn run_launcher(settings: Settings) -> AppResult<()> {
doctor_view
.buffer()
.set_text(&diagnostics_service::doctor_report());
status_label.set_text(&result.message);
let status_prefix = match (result.attempted, result.repaired) {
(true, true) => "Repair complete",
(true, false) => "Repair attempted",
(false, true) => "No repair needed",
(false, false) => "Repair unavailable",
};
status_label.set_text(&format!("{status_prefix}: {}", result.message));
});
}
connect_clear_shortcut(
Expand Down
27 changes: 26 additions & 1 deletion src/app/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ pub fn run(capture: CaptureResult, settings: Settings, current_mode: CaptureMode
close_after_copy_toggle.set_active(settings.borrow().close_after_copy);
close_after_copy_toggle
.set_tooltip_text(Some("Close the editor immediately after Ctrl+C or Copy."));
let close_after_save_toggle = gtk::CheckButton::with_label("Close After Save");
close_after_save_toggle.set_active(settings.borrow().close_after_save);
close_after_save_toggle
.set_tooltip_text(Some("Close the editor immediately after Save or Save As."));
let open_after_save_toggle = gtk::CheckButton::with_label("Open After Save");
open_after_save_toggle.set_active(settings.borrow().open_after_save);
open_after_save_toggle
Expand Down Expand Up @@ -278,15 +282,20 @@ pub fn run(capture: CaptureResult, settings: Settings, current_mode: CaptureMode
let c = canvas.clone();
let s = Rc::clone(&settings);
let status_label = status_label.clone();
let app = app.clone();
save_btn.connect_clicked(move |_| {
if let Ok(png) = c.render_png() {
let cfg = s.borrow();
if let Ok(path) = export::save_capture(&png, &cfg, current_mode) {
let close_after_save = cfg.close_after_save;
c.mark_saved();
let _ = export::maybe_open_saved_path(&path, &cfg);
status_label
.set_text(&format!("Saved annotated image to {}.", path.display()));
eprintln!("Saved annotated image: {}", path.display());
if close_after_save {
app.quit();
}
Comment on lines +285 to +298
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Please add coverage for the new close-after-save branches.

Both save paths now have extra quit behavior, but there’s no inline regression coverage for either branch. Even a small extracted post-save helper would make this much easier to test and would keep future save-flow changes safer. As per coding guidelines, "Add Rust unit tests inline under #[cfg(test)] blocks near the code they validate" and "Write focused Rust unit tests for parser behavior, save-path logic, and error recovery when changing those flows".

Also applies to: 803-845

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/window.rs` around lines 285 - 298, The save click handler's new
close-after-save branch isn't covered by tests; extract the post-save behavior
into a small helper function (e.g., handle_post_save or post_save_actions) that
takes the saved path, cfg (or the close_after_save bool) and app handle, move
the logic that calls c.mark_saved(), export::maybe_open_saved_path(), sets
status_label text and calls app.quit() into that helper, and then update the
save_btn.connect_clicked closure to call this helper; add #[cfg(test)] unit
tests next to the helper that exercise both branches (close_after_save = true
and false), verifying that mark_saved and maybe_open_saved_path are invoked and
that app.quit() would be triggered only when expected (use a testable app stub
or flag to observe quit).

} else {
status_label.set_text("Saving failed. Check the terminal for details.");
}
Expand All @@ -312,6 +321,14 @@ pub fn run(capture: CaptureResult, settings: Settings, current_mode: CaptureMode
let _ = settings_service::save(&guard);
});
}
{
let s = Rc::clone(&settings);
close_after_save_toggle.connect_toggled(move |toggle| {
let mut guard = s.borrow_mut();
guard.close_after_save = toggle.is_active();
let _ = settings_service::save(&guard);
});
}
{
let s = Rc::clone(&settings);
open_after_save_toggle.connect_toggled(move |toggle| {
Expand Down Expand Up @@ -491,6 +508,7 @@ pub fn run(capture: CaptureResult, settings: Settings, current_mode: CaptureMode
inspector.append(&export_settings_title);
inspector.append(&folder_btn);
inspector.append(&close_after_copy_toggle);
inspector.append(&close_after_save_toggle);
inspector.append(&open_after_save_toggle);

shell.append(&tools_panel);
Expand All @@ -513,8 +531,9 @@ pub fn run(capture: CaptureResult, settings: Settings, current_mode: CaptureMode
let s = Rc::clone(&settings);
let status_label = status_label.clone();
let window = window.clone();
let app = app.clone();
save_as_btn.connect_clicked(move |_| {
prompt_save_as(&window, &c, &s, &status_label, current_mode);
prompt_save_as(&window, &c, &s, &status_label, &app, current_mode);
});
}
{
Expand Down Expand Up @@ -786,6 +805,7 @@ fn prompt_save_as(
canvas: &EditorCanvas,
settings: &Rc<RefCell<Settings>>,
status_label: &gtk::Label,
app: &adw::Application,
current_mode: CaptureMode,
) {
let cfg = settings.borrow().clone();
Expand All @@ -804,6 +824,7 @@ fn prompt_save_as(
let canvas = canvas.clone();
let settings = Rc::clone(settings);
let status_label = status_label.clone();
let app = app.clone();
dialog.run_async(move |dialog, response| {
if response == gtk::ResponseType::Accept {
if let Some(file) = dialog.file() {
Expand All @@ -813,11 +834,15 @@ fn prompt_save_as(
Ok(()) => {
canvas.mark_saved();
let cfg = settings.borrow();
let close_after_save = cfg.close_after_save;
let _ = export::maybe_open_saved_path(&path, &cfg);
status_label.set_text(&format!(
"Saved annotated image to {}.",
path.display()
));
if close_after_save {
app.quit();
}
}
Err(err) => {
status_label.set_text(&format!("Save As failed: {err}"));
Expand Down
2 changes: 1 addition & 1 deletion src/capture/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::platform::linux::{grim, hyprctl};
use crate::settings::config::CaptureMode;
use anyhow::Result;

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct CaptureResult {
pub png_data: Vec<u8>,
}
Expand Down
71 changes: 71 additions & 0 deletions src/capture/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,74 @@ pub fn capture(backend: &dyn CaptureBackend) -> Result<CaptureResult> {
.context("grim failed to capture active window region")?;
Ok(CaptureResult { png_data })
}

#[cfg(test)]
mod tests {
use super::capture;
use crate::capture::CaptureBackend;
use anyhow::{anyhow, Result};
use std::cell::RefCell;

#[derive(Default)]
struct FakeBackend {
calls: RefCell<Vec<&'static str>>,
geometry_result: Option<String>,
region_result: Option<Vec<u8>>,
}

impl CaptureBackend for FakeBackend {
fn capture_fullscreen(&self) -> Result<Vec<u8>> {
Ok(vec![])
}

fn capture_region(&self, _geometry: &str) -> Result<Vec<u8>> {
self.calls.borrow_mut().push("capture_region");
self.region_result
.clone()
.ok_or_else(|| anyhow!("region capture failed"))
}

fn active_window_geometry(&self) -> Result<String> {
self.calls.borrow_mut().push("active_window_geometry");
self.geometry_result
.clone()
.ok_or_else(|| anyhow!("geometry lookup failed"))
}
}

#[test]
fn uses_geometry_then_region_capture() {
let backend = FakeBackend {
geometry_result: Some("5,6 7x8".to_string()),
region_result: Some(vec![9, 8, 7]),
..FakeBackend::default()
};

let result = capture(&backend).unwrap();

assert_eq!(result.png_data, vec![9, 8, 7]);
assert_eq!(
backend.calls.borrow().as_slice(),
&["active_window_geometry", "capture_region"]
);
}

#[test]
fn returns_geometry_errors_without_region_capture() {
let backend = FakeBackend {
geometry_result: None,
region_result: Some(vec![1, 2, 3]),
..FakeBackend::default()
};

let err = capture(&backend).unwrap_err();

assert!(err
.to_string()
.contains("failed to resolve active window geometry from Hyprland"));
assert_eq!(
backend.calls.borrow().as_slice(),
&["active_window_geometry"]
);
}
}
10 changes: 3 additions & 7 deletions src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,6 @@ pub struct PortalRepairResult {
pub message: String,
}

pub fn auto_repair_portals() -> PortalRepairResult {
repair_portals_with_path(env::var_os("PATH"))
}

pub fn repair_portals() -> PortalRepairResult {
repair_portals_with_path(env::var_os("PATH"))
}
Expand Down Expand Up @@ -382,11 +378,11 @@ fn colorize_doctor_report(report: &str) -> String {
let colored = if line == "Kiekje Doctor Report" || line == "Desktop Portal Report" {
format!("\x1b[1;36m{line}\x1b[0m")
} else if line.starts_with("[OK]") {
format!("\x1b[32m{}\x1b[0m", line.replacen("[OK]", "[OK]", 1))
format!("\x1b[32m{line}\x1b[0m")
} else if line.starts_with("[MISS]") {
format!("\x1b[33m{}\x1b[0m", line.replacen("[MISS]", "[MISS]", 1))
format!("\x1b[33m{line}\x1b[0m")
} else if line.starts_with("[WARN]") {
format!("\x1b[35m{}\x1b[0m", line.replacen("[WARN]", "[WARN]", 1))
format!("\x1b[35m{line}\x1b[0m")
} else if line.trim_start().starts_with("Install:")
|| line.trim_start().starts_with("Hint:")
{
Expand Down
21 changes: 12 additions & 9 deletions src/editor/canvas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,16 +526,19 @@ fn cairo_surface_to_rgba_image(surface: &mut cairo::ImageSurface) -> Result<Rgba
let g = data[offset + 1] as u16;
let r = data[offset + 2] as u16;
let a = data[offset + 3] as u16;
let pixel = if a == 0 {
[0, 0, 0, 0]
} else {
[
((r * 255) / a).min(255) as u8,
((g * 255) / a).min(255) as u8,
((b * 255) / a).min(255) as u8,
a as u8,
]
let unpremultiply = |channel: u16| {
channel
.saturating_mul(255)
.checked_div(a)
.unwrap_or(0)
.min(255) as u8
};
let pixel = [
unpremultiply(r),
unpremultiply(g),
unpremultiply(b),
a as u8,
];
image.put_pixel(x, y, Rgba(pixel));
}
}
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ fn print_settings(settings: &Settings) {
);
println!("copy_to_clipboard: {}", settings.copy_to_clipboard);
println!("close_after_copy: {}", settings.close_after_copy);
println!("close_after_save: {}", settings.close_after_save);
println!("open_after_save: {}", settings.open_after_save);
println!("open_editor: {}", settings.open_editor);
println!(
Expand Down
16 changes: 8 additions & 8 deletions src/platform/linux/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@

use super::grim::{GrimCli, ScreenshotTool};
use super::hyprctl::{ActiveWindowGeometrySource, HyprctlCli};
use crate::capture::CaptureBackend;
use anyhow::Result;

pub trait LinuxCaptureBackend {
fn capture_fullscreen(&self) -> Result<Vec<u8>>;
fn capture_region(&self, geometry: &str) -> Result<Vec<u8>>;
fn active_window_geometry(&self) -> Result<String>;
}

#[derive(Debug, Clone, Default)]
pub struct GrimHyprlandBackend<S = GrimCli, W = HyprctlCli> {
screenshot_tool: S,
Expand All @@ -25,7 +20,11 @@ impl<S, W> GrimHyprlandBackend<S, W> {
}
}

impl<S, W> LinuxCaptureBackend for GrimHyprlandBackend<S, W>
pub fn current_backend() -> GrimHyprlandBackend {
GrimHyprlandBackend::new(GrimCli, HyprctlCli)
}

impl<S, W> CaptureBackend for GrimHyprlandBackend<S, W>
where
S: ScreenshotTool,
W: ActiveWindowGeometrySource,
Expand All @@ -45,7 +44,8 @@ where

#[cfg(test)]
mod tests {
use super::{GrimHyprlandBackend, LinuxCaptureBackend};
use super::GrimHyprlandBackend;
use crate::capture::CaptureBackend;
use crate::platform::linux::grim::ScreenshotTool;
use crate::platform::linux::hyprctl::ActiveWindowGeometrySource;
use anyhow::Result;
Expand Down
1 change: 1 addition & 0 deletions src/platform/linux/grim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub fn capture_region(geometry: &str) -> Result<Vec<u8>> {
GrimCli.capture_region(geometry)
}

#[allow(dead_code)]
pub fn capture_fullscreen() -> Result<Vec<u8>> {
GrimCli.capture_fullscreen()
}
4 changes: 4 additions & 0 deletions src/services/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ pub fn run(mode: CaptureMode, settings: &Settings) -> AppResult<CaptureExecution
)
}

#[expect(
clippy::too_many_arguments,
reason = "test hook injection keeps the production workflow free of trait-object plumbing"
)]
Comment on lines +37 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# Check if Cargo.toml exists and view its current contents
cat Cargo.toml | head -30

Repository: remcostoeten/kiekje

Length of output: 832


🏁 Script executed:

# Check if src/services/capture.rs exists and view lines 37-40
if [ -f "src/services/capture.rs" ]; then
  echo "File exists. Viewing lines 37-40:"
  sed -n '37,40p' src/services/capture.rs
else
  echo "File not found at src/services/capture.rs"
  # Try to find it
  find . -name "capture.rs" -type f
fi

Repository: remcostoeten/kiekje

Length of output: 237


🌐 Web query:

Rust #[expect] macro stabilization version release

💡 Result:

Rust’s #[expect] macro/attribute for lints (from RFC 2383 “lint_reasons”) was stabilized in Rust 1.81.0.[1][2] The Rust 1.81.0 release blog states that “1.81 stabilizes … a new lint level, expect” and introduces #[expect(clippy::…)] for expecting a lint to occur, warning if it doesn’t.[1] The official Rust 1.81.0 release notes also include “Stabilize #[expect] for lints (RFC 2383)”.[2] For context: the stabilization work corresponds to the “Let’s #[expect] some lints: Stabilize lint_reasons” PR, and that PR lists milestone 1.81.0.[3]

Citations:


Add rust-version to Cargo.toml to declare Rust 1.81 requirement.

#[expect] was stabilized in Rust 1.81 and allows explicitly noting that a particular lint should occur, warning if it doesn't. The project currently has no rust-version key in Cargo.toml, so contributors using a toolchain older than 1.81 will get an opaque compile error. Pinning the MSRV makes the requirement explicit.

📌 Proposed addition to `Cargo.toml`
 edition = "2021"
+rust-version = "1.81"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/capture.rs` around lines 37 - 40, Add a rust-version = "1.81"
entry to the project's Cargo.toml so toolchains prior to Rust 1.81 (which don't
support the stabilized #[expect] attribute used in src/services/capture.rs) will
fail with a clear message; update the top-level [package] section to include
rust-version = "1.81" to declare the MSRV requirement.

fn run_with_hooks<Req, Sleep, Capture, Validate, Copy, Save>(
mode: CaptureMode,
settings: &Settings,
Expand Down
Loading
Loading