From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id ouL2ILo4GmDNbwAA0tVLHw (envelope-from ) for ; Wed, 03 Feb 2021 05:46:34 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id QBwUHLo4GmDeKAAAbx9fmQ (envelope-from ) for ; Wed, 03 Feb 2021 05:46:34 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id CB6939403C9 for ; Wed, 3 Feb 2021 05:46:33 +0000 (UTC) Received: from localhost ([::1]:60748 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l7Azn-0006ZK-Qh for larch@yhetil.org; Wed, 03 Feb 2021 00:46:31 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:43786) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l7Az6-0006YX-0A for emacs-orgmode@gnu.org; Wed, 03 Feb 2021 00:45:48 -0500 Received: from out0.migadu.com ([2001:41d0:2:267::]:39924) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l7Az1-000549-Nu for emacs-orgmode@gnu.org; Wed, 03 Feb 2021 00:45:47 -0500 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kyleam.com; s=key1; t=1612331134; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=TdKsBCMEjm+cCAiILur4r3GeasS5DFIMxdQYaIFt+2A=; b=p2son77TwVDUXOnMrblma5OY2tBORj4vcTsp05BjZMt/0IPJgn7U2Y1zHCSp3h03rL/GHP s4AGRB9zTCdQflBKVguHIpFex1SnrgIpdg2N+MkgOYdURtkK3Z9+JOxt07HsQzj1gAN5Tq 8PlJdt27l/uZNytUYXT07hKzbaQkRJSnS/ZaV0UK9sv7L0jIVvXAH+lCBwKxeBVJ38HxB3 hI52bV/7AhHbsWko3htSdqoUBRuedO38jj9UmNyutXIPEiHCSzq1DuI7EACVethFMk/0ek SIXMzbIr3kghMn+RVrZY9zQ0dBXxkYFZ3XbE+JoFjkOs9oIPw6f0GQHZJZK/Bg== From: Kyle Meyer To: Allen Li Subject: Re: [PATCH] Query when exiting with running clock In-Reply-To: <80o8hu356e.fsf@felesatra.moe> References: <80o8hu356e.fsf@felesatra.moe> Date: Wed, 03 Feb 2021 00:45:33 -0500 Message-ID: <878s85ya9u.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain X-Migadu-Auth-User: kyle@kyleam.com Received-SPF: pass client-ip=2001:41d0:2:267::; envelope-from=kyle@kyleam.com; helo=out0.migadu.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode@gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -2.56 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=kyleam.com header.s=key1 header.b=p2son77T; dmarc=none; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Queue-Id: CB6939403C9 X-Spam-Score: -2.56 X-Migadu-Scanner: scn0.migadu.com X-TUID: PoAiYKEuElUI Allen Li writes: > This is a patch adding a query function when exiting Emacs, warning the > user if there is a running clock. This is useful for preventing the > user from accidentally leaving dangling clocks. I have had success > using a modified personal version of this code. Thanks. I'd find this useful as well. > My personal version instead prompts the user to clock out and save the > buffer. I didn't use it for the patch because it seems a little hacky > to me; e.g., kill-emacs-query-functions doesn't mention whether > functions can have side effects, and the UI is inconsistent with > save-buffers-kill-emacs. The code for my personal version: Fwiw that's what Emacs's lisp/calendar/timeclock.el does: (defun timeclock-query-out () "Ask the user whether to clock out. This is a useful function for adding to `kill-emacs-query-functions'." (and (equal (car timeclock-last-event) "i") (y-or-n-p "You're currently clocking time, clock out? ") (timeclock-out)) ;; Unconditionally return t for `kill-emacs-query-functions'. t) > Subject: [PATCH 1/2] org-clock: Replace org-clocking-buffer with > org-clock-is-active > > org-clocking-buffer and org-clock-is-active have the same definition. > org-clocking-buffer is defined in org-clock.el while > org-clock-is-active is defined in org.el. org-clock.el requires > org.el. > > org-clocking-buffer is kept as an alias to preserve backward > compatibility with any user code. Dropping the duplicate definitions using an alias sounds good, though as I mention below I'd prefer the spots that are specifically concerned with a buffer to keep using the org-clocking-buffer name. > * lisp/org-clock.el (org-clocking-buffer): Changed to alias. > (org-clocking-p): Changed reference to org-clocking-buffer. > (org-clock-out): Changed reference to org-clocking-buffer. > (org-clock-cancel): Changed reference to org-clocking-buffer. > (org-clock-sum): Changed reference to org-clocking-buffer. > (org-clock-out-if-current): Changed reference to org-clocking-buffer. > (org-clock-save): Changed reference to org-clocking-buffer. > --- > lisp/org-clock.el | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/lisp/org-clock.el b/lisp/org-clock.el > index 892ae1810..37459580b 100644 Hmm, this patch doesn't apply to the current master (041459727). The preimage blob is quite old: $ git describe 892ae1810 release_8.2.7b-7-gbacfe5b4f:lisp/org-clock.el $ git log --oneline --format="%h %cd" --find-object=892ae1810 ca21b7b86 Mon Jan 12 12:35:10 2015 +0100 bacfe5b4f Fri Jul 25 11:02:55 2014 +0200 > --- a/lisp/org-clock.el > +++ b/lisp/org-clock.el > @@ -503,13 +503,11 @@ of a different task.") > (mapc (lambda (m) (org-check-and-save-marker m beg end)) > org-clock-history)) > > -(defun org-clocking-buffer () > - "Return the clocking buffer if we are currently clocking a task or nil." > - (marker-buffer org-clock-marker)) > +(defalias 'org-clocking-buffer #'org-clock-is-active) > > (defun org-clocking-p () > "Return t when clocking a task." > - (not (equal (org-clocking-buffer) nil))) > + (not (equal (org-clock-is-active) nil))) Eh, so this too could just be an alias to org-clock-is-active, or, if it looks like any callers really depend on this being t rather than just non-nil (but why would they?), `(and (org-clock-is-active) t)' would be better. Anyway, I know you're just doing the plain substitution here, so I'm fine keeping it at that. > > (defvar org-clock-before-select-task-hook nil > "Hook called in task selection just before prompting the user.") > @@ -1501,7 +1499,7 @@ to, overriding the existing value of `org-clock-out-switch-to-state'." > ts te s h m remove) > (setq org-clock-out-time now) > (save-excursion ; Do not replace this with `with-current-buffer'. > - (org-no-warnings (set-buffer (org-clocking-buffer))) > + (org-no-warnings (set-buffer (org-clock-is-active))) This is an example of a spot that I think should continue using the org-clocking-buffer variant for readability. And scanning the other spots in this patch, I think they actually all fall into this category. > Subject: [PATCH 2/2] org-clock: Query when exiting with running clock > > It's annoying to accidentally quit Emacs with a running clock, then > resolve the clock the next time when Emacs is started. > > * lisp/org-clock.el (org-clock-kill-emacs-query): New function. > --- > lisp/org-clock.el | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/lisp/org-clock.el b/lisp/org-clock.el > index 37459580b..bc1762f63 100644 > --- a/lisp/org-clock.el > +++ b/lisp/org-clock.el > @@ -2886,6 +2886,15 @@ The details of what will be saved are regulated by the variable > (if (outline-invisible-p) > (org-show-context)))))))))) > > +(defun org-clock-kill-emacs-query () > + "Query user when killing Emacs. > +This function is added to `kill-emacs-query-functions'." > + (if (org-clocking-buffer) Hmm, why use org-clocking-buffer rather than org-clock-is-active? After a patch that introduced org-clock-is-active and switched a bunch of spots over to it, I would have expected you to use it here. > + (y-or-n-p "Org clock is running; exit anyway? ") > + t)) > + > +(add-hook 'kill-emacs-query-functions #'org-clock-kill-emacs-query) lisp/calendar/timeclock.el has the following option: (defcustom timeclock-ask-before-exiting t "If non-nil, ask if the user wants to clock out before exiting Emacs. This variable only has effect if set with \\[customize]." :set (lambda (symbol value) (if value (add-hook 'kill-emacs-query-functions #'timeclock-query-out) (remove-hook 'kill-emacs-query-functions #'timeclock-query-out)) (set symbol value)) :type 'boolean) I wonder if we should do something similar.