feat(ui): confirm on exit when quitting last sheet (q) and quit-all (gq)#2891
feat(ui): confirm on exit when quitting last sheet (q) and quit-all (gq)#2891denisotree wants to merge 3 commits intosaulpw:developfrom
Conversation
|
Hi @denisotree, thanks for the PR. Doesn't the |
|
Hi @saulpw, I understand this seems like a duplication, but I've often encountered this situation: I'm exploring a CSV in Visidata, I've already labeled the columns, I've done groupings, and sometimes I accidentally press q too many times and exit the program. It's very frustrating. I'd be happy if the program would always warn me that pressing q would kick me out to the console. |
|
Do you have |
|
Hi! My change handles the specific case where there’s only one sheet left before returning to the console, or when quitting the entire console with g + q. It prevents the full column frame and computed values from being reset when q is pressed accidentally. In this situation, options.quitguard = True doesn’t affected. An alternative (and probably cleaner) approach would be to adjust confirmQuit to trigger whenever len(vd.stackedSheets) <= 1. What do you think? |
- Consolidated exit confirmation checks into single confirmQuit method - Removed redundant quit_with_exit_confirm and quit_all_with_exit_confirm functions - Added check for last sheet exit confirmation in main confirmQuit flow - Updated 'q' and 'gq' command bindings to use direct quit calls - Simplified code paths while maintaining all existing confirmation behaviors
- Simplified conditional structure by checking vd._nextCommands first and returning early - Reordered quit confirmation checks to prioritize program exit case (single sheet) - Combined duplicate vd.draw_all() calls into single call at start of function - Updated confirmation message format for consistency across all cases - Removed redundant vd._nextCommands check in final condition
|
I’ve updated the confirmQuit function to what I think is the ideal flow. You’re right — VisiData tends to be a bit too aggressive about prompting for quit confirmations when options.quitguard = True, even for minimal changes. The new implementation keeps your original logic intact, but now a quit confirmation is only triggered when pressing q would actually exit the console. |
| return | ||
| vd.draw_all() | ||
| if len(vd.stackedSheets) <= 1: | ||
| vd.confirm(f'Are you sure you want to {verb}?') |
There was a problem hiding this comment.
This will always ask for confirmation before quitting. I can accept that if the sheet has been modified OR if options.quitguard is True, but we need to launch vd and press gq immediately to exit without confirmation. If you want, lobby for options.quitguard to be set to True by default, but we can't hardcode it like this.
| if vs.options.quitguard and vs.precious and vs.hasBeenModified and not vd._nextCommands: | ||
| vd.draw_all() | ||
| if vd._nextCommands: | ||
| return |
There was a problem hiding this comment.
This means that having commands queued up means the quit is auto-confirmed, since vd.confirm raises an Exception to indicate disconfirmation.
Summary
qon the last stacked sheet (app exit)gq(quit all sheets)Rationale
User-facing behavior
q:gq: shows confirmation before quitting all sheetsImplementation
q→ quit_with_exit_confirm()gq→ quit_all_with_exit_confirm()CAA