From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0.migadu.com ([2001:41d0:403:58f0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id WEMtKMB2mmaVFgAAqHPOHw:P1 (envelope-from ) for ; Fri, 19 Jul 2024 14:22:56 +0000 Received: from aspmx1.migadu.com ([2001:41d0:403:58f0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0.migadu.com with LMTPS id WEMtKMB2mmaVFgAAqHPOHw (envelope-from ) for ; Fri, 19 Jul 2024 16:22:56 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=PQatF3bl; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1721398976; h=from:from:sender:sender: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:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=15C78fLByKOrGiKTU0svxlbi4RQLy8TrX7AaArky1V4=; b=boGj+V8oHIoKt02DkUlVmZhU4mssuV+1U/4m7yZWX8hMyxxMn9R3KdWEBVFcW2V9o/GOR2 PDDveIa4Yf9qlo8o0t5weXZA9epsBB7LGRmklstYze5KeSzPwGtqivySGTLVoBSJKrVfx6 d0VGyI6KpznxBNMPB/t8Fn21TdotZK+cyJjemAyBSFn60/9Rn+XsZWvNVNsYGvtAFbo/dA hqEqCGV+3fwB7xdh1Lf6LeTqCHHssbrVI5CnIDbssWpJdT2tFI7wEWEtJSkuUoxtpw63Sv Ak+dp8SLvnfzpuwPwyZJnvE2HJwQ17eYT8voM1prWyBxVHwWpuJ8T/ZvAwwmgg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1721398976; a=rsa-sha256; cv=none; b=SA68SWs7YN8zJYAN+rxCYXE4kWKGDniC/gjiPbHFGXL3hpdED1nyeeMNoTtlry0zSt5ozy x06biVy01lbUU4ZK+m+aMvF9PWvRniCF2u4Rhszu58kbGQopJoSdQOWpjBukqaAyZLkTTu FSau583StnmwMQiMhjfFQlZvI+XO+h3ukvQCj8NTWT3riF+LKCxIUyaxx2Zw4ae+WCsL7E uVJjUc6wlO8FC1qkKUX+NhmGKShdt8ZJvH1m2gmbWgBwWjqwYIKOB+N3dW7/FTf9REBMG2 813s8HAM6u/mu0gTpNHhR/IOOSSvaOB3QsLBRIF7dFo14d6cYlxKpj5eRGC5gw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=PQatF3bl; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" 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 35F37640B3 for ; Fri, 19 Jul 2024 16:22:56 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sUoUk-0000Ra-43; Fri, 19 Jul 2024 10:22:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sUoUh-0000RN-40 for emacs-orgmode@gnu.org; Fri, 19 Jul 2024 10:21:59 -0400 Received: from mout01.posteo.de ([185.67.36.65]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sUoUe-0002lm-K2 for emacs-orgmode@gnu.org; Fri, 19 Jul 2024 10:21:58 -0400 Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 1AFFA240028 for ; Fri, 19 Jul 2024 16:21:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1721398913; bh=MRtb3ZWA3349BK5A/NqO6D9d4R/C5I/UbsdZuQLcRCA=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type: From; b=PQatF3blVl7YeqmmBoP4Toci48480N0FwKMLj1br9PkaibzuDRbyhROh21jDpktaS ZqrgLU2OvO6pRGWROs+ff7wPFwqVK2Q2Y1T7xW1n1zQt9AJd+cUdZzLfw4dMXTy0Wa X18Gn34SjdL+pNiakKncM98kQzrYDjdWbViUwm5ZqRujC649q92w0dL6ID8AKquISc xt2x227/Bcrg3d1lF6EniXqcci3AQgMPwdccNH4cdNAyobrmTClHcUQjjhT/u7Sla9 Wh8Jh+yICuoaH/abuMLc88wK4jQAICwzhpWAc6Ar0zudExp8cQgu+SmwXmHiQaAqv4 GOtcOuh5MS8zA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4WQX1D3Fn0z6tvJ; Fri, 19 Jul 2024 16:21:52 +0200 (CEST) From: Ihor Radchenko To: Bruno Barbier Cc: emacs-orgmode , Jack Kamm , Matt Subject: Re: Pending contents in org documents In-Reply-To: <6698ccdb.050a0220.63d89.3c1c@mx.google.com> References: <87o7d0mm54.fsf@localhost> <87plvpjj76.fsf@localhost> <65fc06c1.5d0a0220.0d53.efdc@mx.google.com> <87frwjlr1a.fsf@localhost> <6601b872.050a0220.31a67.5a55@mx.google.com> <87le63c3qy.fsf@localhost> <660ed63d.050a0220.36fdd.af23@mx.google.com> <87cyqwyvgw.fsf@localhost> <66225435.5d0a0220.f60e4.c590@mx.google.com> <87r0f02vq2.fsf@localhost> <664f6f54.050a0220.10342.b002@mx.google.com> <87o78vwnds.fsf@localhost> <6658cd1f.050a0220.f6605.de1a@mx.google.com> <87jzja71n7.fsf@localhost> <665abf9f.050a0220.d2a6e.ef6b@mx.google.com> <87plsyz3rn.fsf@localhost> <666d4779.050a0220.13efd.8a2d@mx.google.com> <87ed8xi688.fsf@localhost> <668a5cc4.df0a0220.11fdd.05c1@mx.google.com> <87msmtflhq.fsf@localhost> <6698ccdb.050a0220.63d89.3c1c@mx.google.com> Date: Fri, 19 Jul 2024 14:23:22 +0000 Message-ID: <87y15x4g11.fsf@localhost> MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=185.67.36.65; envelope-from=yantar92@posteo.net; helo=mout01.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 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, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, 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.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US X-Migadu-Spam-Score: -5.14 X-Spam-Score: -5.14 X-Migadu-Queue-Id: 35F37640B3 X-Migadu-Scanner: mx11.migadu.com X-TUID: WruHT0Uye8nk Bruno Barbier writes: >>> I added a function 'org-pending-unlock-NOW!' which unlock the region >>> immediately. The uppercase "NOW!" emphasizes that it's not the >>> "safe" way to unlock a region. >> >> I expect to see this function called by some kind of button in the >> details buffer, so that users can actually call it. > > This function should not be used, not even in code, except to work > around bugs. I would prefer not to provide a button for it. Then, there is no point in this function - users will never know about it. Maybe you do expose it as a button, but also supply a yes/no prompt asking for confirmation? >> We should provide a user option to suppress the query. Something like >> what `confirm-kill-processes' does. Maybe even support something akin >> `process-query-on-exit-flag', but for reglocks. > > IMHO, it's the package that uses org-pending that should provide such a > feature: I may not care about loosing data by ignoring on-going code > block executions, but, I'll probably care if Emacs aborts some money > transfers. I kindly disagree. I am not proposing anything dramatically different from what Emacs already does. If you look into `confirm-kill-process' docstring, you will see that Emacs does, in fact, have mechanisms for Elisp packages to tell whether a given process is important or not (`process-query-on-exit-flag'). Yet, `confirm-kill-processes' overrides that mechanism if Emacs user wishes to and sets the value to non-default nil. There is nothing principally different in org-pending users compared to processes. So, in order to conform with the rest of Emacs API, we do need to provide an equivalent user customization. We will not suppress queries by default, but we ought to give users an option to suppress these queries. Maybe even follow `confirm-kill-process' setting in how org-pending behaves. >> Please describe what the default value of ON-OUTCOME (when ON-OUTCOME is >> not explicitly provided) does right in the docstring. > > Done. Thanks! There is one typo in the relevant commit: + The default ON-OUTCOME +function replaces the region on success and ignore failures; in all ^ignores > I've pushed the changes to my repository. Continuing the code review. > (defface org-pending-outcome-failure > ... > (defface org-pending-outcome-success If possible, please derive the faces from either org-faces.el or from built-in faces like the ones listed in `modus-themes-faces' variable. Ideally, we should have no hard-coded color names. > :version "30.1" Please, use :package-version instead of :version. > (defun org-pending--status-still-pending-p (status) This function is trivial and used only once. Maybe you can just inline it. > (defvar org-pending--outcome-keymap > (defvar org-pending--pending-keymap Why are these keymaps internal? May we better expose them to users for modification? It may also be a good idea to add some actual keyboard binding to this map. > (defun org-pending--popup-failure-details (exc) This _internal_ function is unused in org-pending.el, but instead used in ob-core.el. I am not yet looking into the changes outside org-pending, but it is generally not ideal when library users need to call an internal function from the library. Also, this particular function does not look relevant to org-pending library per se. It is rather generic and may be inlined. > (defun org-pending-reglock-live-p (reglock) You introduce this API function, but yet using `org-pending-reglock--get-live-p' directly in other places in the library. It will be more readable to use `org-pending-reglock-live-p' everywhere. > (defun org-pending-reglock-duration (reglock) The docstring does not mention REGLOCK argument. > (defun org-pending-reglock-property (reglock prop) ... PROP argument. > (defun org-pending-reglock-set-property (reglock prop val) > "See `org-pending-reglock-property'." May as well write the full docstring. > (defun org-pending-reglock-delete-outcome-marks (reglock) > "Delete visual hints of the outcome for this REGLOCK, if any. > Do nothing if the outcome is not known. Do nothing if there are no We use double space between sentences in the docstrings. > (cl-defun org-pending (region > &key anchor name > (on-outcome #'org-pending-on-outcome-replace)) NAME key is not documented in the docstring. (and, more generally, please do M-x checkdoc on org-pending.el) > (user-error "This region lock has been destroyed.")))) Error messages should not end with period. > (let ((buffer (get-buffer-create "*Region Lock*"))) Please avoid using constant strings. Instead, declare them as defvar. > (defun org-pending-describe-reglock (reglock) > "Describe REGLOCK in a buffer. > > Describe position REGLOCK. > The information is displayed in new buffer. in *a* new buffer... except that it is not really a new buffer, when another REGLOCK buffer is already being displayed. May users want to see info about multiple reglocks at the same time? > (setq-local header-line-format > (format "Lock info (at %s), hit 'g' to update." > (format-time-string "%T"))) Please, avoid using hard-coded bindings. Instead, see `substitute-command-keys' (also see how org-capture.el sets up header line) > (defun org-pending--describe-reglock-at-point () Why internal? It is a command users may want to call via M-x. > (defun org-pending-pre-insert-outcome-default (_lock _message) > "Default value for `org-pending-pre-insert-outcome-function'.") May as well just use #'ignore > (defvar org-pending-pre-insert-outcome-function > (defvar org-pending-post-insert-outcome-function Should these be defcustom ? > (overlay-put outcome-ovl > 'help-echo > "Last lock outcome, click to popup its full description.") This assumes fixed key bindings in the lock overlay keymap. Which may or may not be the case. Please compute 'help-echo programmatically, based on the key map. (using `substitute-command-keys'). > (cl-defun org-pending--update (reglock status data) No docstring. Please, add. > (let* ((reg (org-pending-reglock-region reglock)) > (start (car reg)) > (end (cdr reg)) > (buf (marker-buffer start))) > (when (buffer-live-p buf) > (with-current-buffer buf > (save-excursion > (if (> (- end start) 1) What if the buffer is narrowed and reglock is outside the narrowing? > (with-current-buffer buffer > (without-restriction `without-restriction' is only available since Emacs 29. However, we still support Emacs 27 and will support Emacs 28 for quite a while. So, please avoid `without-restriction' if possible. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at . Support Org development at , or support my work at