Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 28, 2025

Pressing 'q' in the main view previously required two confirmations regardless of whether items were marked for deletion. This added unnecessary friction when no destructive action was pending.

Changes

Modified handle_quit() in src/interactive/app/eventloop.rs:

  • Quit instantly when window.mark_pane.is_none() (no deletion risk)
  • Preserve two-press confirmation when items are marked (safety measure for destructive operations)
Main => {
    if self.glob_navigation.is_some() {
        self.quit_glob_mode(tree_view, window);
    } else if window.mark_pane.is_none() {
        // Quit instantly when nothing is marked for deletion
        return Some(Ok(WalkResult {
            num_errors: self.stats.io_errors,
        }));
    } else if !self.pending_exit {
        self.pending_exit = true;
    } else {
        return Some(Ok(WalkResult {
            num_errors: self.stats.io_errors,
        }));
    }
}

Added tests covering both instant quit and two-press scenarios.

Original prompt

When pressing 'q' and nothing is selected for deletion, quit instantly.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix instant quit when pressing 'q' with no selection Quit instantly when no items marked for deletion Oct 28, 2025
Copilot AI requested a review from Byron October 28, 2025 03:49
Copilot finished work on behalf of Byron October 28, 2025 03:49
@Byron Byron force-pushed the copilot/fix-instant-quit-on-q branch from c9455a1 to d0f66d6 Compare October 28, 2025 04:46
@Byron Byron marked this pull request as ready for review October 28, 2025 05:00
@Byron Byron merged commit 938ae33 into main Oct 28, 2025
2 checks passed
[[package]]
name = "home"
version = "0.5.11"
version = "0.5.12"
Copy link

Choose a reason for hiding this comment

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

Note that this version requires Rust 1.88, contrary to your own MSRV 1.83.

You could use CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback when updating your lockfile to avoid this automatically.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the hint!

I would have thought that if Cargo treats rust-version as MSRV, it would automatically assure version updates don't violate it.
Personally, I didn't think of it as MSRV, just as the minimal version that is needed to build the application code, just as a service to those who want to build it themselves.

If dependencies aren't adhering to it, I'd rather remove the field, I think.
Does that make sense?

Copy link

Choose a reason for hiding this comment

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

I would have thought that if Cargo treats rust-version as MSRV, it would automatically assure version updates don't violate it.

That's what the fallback resolver is about, but it's relatively new. You can opt into this more directly starting in 1.84, and it's the default in 2024 edition (1.85+).
https://blog.rust-lang.org/2025/01/09/Rust-1.84.0/#cargo-considers-rust-versions-for-dependency-version-selection

Personally, I didn't think of it as MSRV, just as the minimal version that is needed to build the application code, just as a service to those who want to build it themselves.

If dependencies aren't adhering to it, I'd rather remove the field, I think. Does that make sense?

Even as just a hint, I think it's still useful to keep, but maybe it would be worth adding a CI job for it so you know when it needs to be increased.

I don't think it's a big deal for dependencies to raise their MSRV now that we have a resolver that deals with it, and you don't necessarily need to follow them if you're not using new features too. It's up to you!

Copy link
Owner

Choose a reason for hiding this comment

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

Then I think it's fixed now - I intuitively removed that version field and added edition = 2024. But from what I read here, it seems it can be re-added to protect against new lints showing up unexpectedly.

In any case, thanks for all your help, it was quite inspirational.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants