From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id kB0/Njwsu2bx/QAA62LTzQ:P1 (envelope-from ) for ; Tue, 13 Aug 2024 09:49:49 +0000 Received: from aspmx1.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1.migadu.com with LMTPS id kB0/Njwsu2bx/QAA62LTzQ (envelope-from ) for ; Tue, 13 Aug 2024 11:49:48 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=LzlDQNT2; 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"; dmarc=pass (policy=none) header.from=posteo.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1723542588; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=H8uP8mrw/q6KXtrvAhcR95t9V5Y5AjlYybze7t1L8Fo=; b=XSinZln23UM1TTXkxm37jExyAYWt7FipSwp0hPe3aZsdM0nU9LIY4VhW8itXdmRka8Tm0n 13pv6Oj/AFGsuSFxI5Jh0W2SIDImGgb/TPymweuImHdeFvqoNUTeiYQTXqc/+SL33dqeOX IYD+CspS5I6WTvZO6tjRScxqMPoFTM7FMy68+LMQ1pGw3vwHbP1QCz7J7b10Y8n8XvESEF SigUNqUrep/9JJLvAIHI2Ccp6ea2GoTY7PcuU9WN+em9NZw06LlEYtjkcDVY9rCxuEA9gq ynF1Lym++R8lsFkcJDodcRHHKiKEPAZ238+fV7ujGa1Q8kVUII5FK/srRFBcXg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=LzlDQNT2; 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"; dmarc=pass (policy=none) header.from=posteo.net ARC-Seal: i=1; s=key1; d=yhetil.org; t=1723542588; a=rsa-sha256; cv=none; b=AU+aUAENgiq2ZNkNyBKGson962/yVsNPUUAVa+MbW4O7nlH6c0xs49nBXIG9guAwjsr8b6 5viU116TNZb+7Z6QV4KzA/lipAZcRA0zkBb3lI3szLLYf7Oc8WV0lOlf6tJNcNKAAdfXH6 3BN/Wrp8rZrBlIC+A48c3VQIrBFSFDmEGvZlz5+8kwwAzOZrCsnIAjm9gj0EMJs/vpm1PN 6Pgekem0byNyn/JPUEF8W5QFfKwBM0NhwrsdHxUJGCEqrT2W+Eudnvk3/kq7IUuBvluw/w kfVo152XL9GKej0jtZejqIeN1tmufAru6SXbt8968kWv6y1pNj9RPGsKCsN3rw== 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 62F3D8B2D for ; Tue, 13 Aug 2024 11:49:48 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sdo9I-0003xa-Uz; Tue, 13 Aug 2024 05:49:04 -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 1sdo9D-0003kU-8M for emacs-orgmode@gnu.org; Tue, 13 Aug 2024 05:48:59 -0400 Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sdo99-0001NJ-Vg for emacs-orgmode@gnu.org; Tue, 13 Aug 2024 05:48:58 -0400 Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id C80C6240104 for ; Tue, 13 Aug 2024 11:48:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1723542533; bh=m5WunsI3VtgLnyvOq6Hj2IhZ1TwK3WQkMnzSLmXJYkQ=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type: Content-Transfer-Encoding:From; b=LzlDQNT2JcJtpKBqiiVsvc+2TtTON9nsDkaHAe6XlioAZUecC4WJ/RtMHn/KzcBYK XidGmXoFVCsP82Te1f+qZiXPmBYWuxWkpjSQ7ItTxrh3PXgtjTDdJqyGbLOWT8GaIG YiQqjItucsVVXiX6z/Ds9pOXJP2cNW4buahhxsRcM2neqqWdZzmfYKqMumzK2/uUCU sMF8U9UKC1purzPmpQKK8chY7FGcUSU6ZQGPZWPuE6GzMMBEdnEh1WYkB8yTfs+Cpd o1Gf/ePqr3ehfd9Qretsg9RORsg2jx2ul5DODh9CaTNNdMSfrSKKzwUURGXNWRencx ImZ0b0Sa5nSiA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Wjmmh6ZwNz9rxM; Tue, 13 Aug 2024 11:48: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: <66b9b665.5d0a0220.21f94d.fccc@mx.google.com> References: <87o7d0mm54.fsf@localhost> <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> <87y15x4g11.fsf@localhost> <66a9fa21.5d0a0220.624f7.d3d6@mx.google.com> <87sevm50rg.fsf@localhost> <66b9b665.5d0a0220.21f94d.fccc@mx.google.com> Date: Tue, 13 Aug 2024 09:49:58 +0000 Message-ID: <878qx0iwft.fsf@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=185.67.36.66; envelope-from=yantar92@posteo.net; helo=mout02.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, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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-Country: US X-Migadu-Flow: FLOW_IN X-Spam-Score: -8.17 X-Migadu-Queue-Id: 62F3D8B2D X-Migadu-Scanner: mx10.migadu.com X-Migadu-Spam-Score: -8.17 X-TUID: DEMjYsiDdh6S Bruno Barbier writes: >>> (concat "\\" >>> "This content is pending. " >>> "\\[org-pending-describe-reglock-at-point]" >>> " to know more.")))) >> >> You set a similar help-echo string in two places. It is a good >> candidate to factor things out into a helper function. > > I'm not sure I understand. The two messages for the user are not the > same, the keymaps are not the same. I'll end up with something like this: > > (defun overlay-set-help-with-keys (overlay message) > ...) > > Is this really what you meant ? Hmm. Not really. I misread the strings a bit and thought that they are more similar than they actually are. Aside: it looks like 'help-echo uses `substitute-command-keys' without a need to run it manually. So, we don't really need to do it: 33.19.4 Properties with Special Meanings (Elisp manual) =E2=80=98help-echo=E2=80=99 If text has a string as its =E2=80=98help-echo=E2=80=99 property, = then when you move the mouse onto that text, Emacs displays that string in the echo area, or in the tooltip window (*note Tooltips::), after passing it through =E2=80=98substitute-command-keys=E2=80=99. >>> ;; Hack to detect if our overlay has been copied into an other >>> ;; buffer. >>> (overlay-put overlay 'org-pending--owner (overlay-buffer overlay)) >> >> AFAIU, the sole purpose of this is >> >> > ;; Try to detect and delete overlays that have been wrongly copied >> > ;; from other buffers. >> >> ... but cloning buffer does not copy its overlays. Or do I miss somethin= g? > > If the function 'make-indirect-buffer' is called with a non-nil CLONE > argument (like `org-tree-to-indirect-buffer' does), the buffer initial > values are copied, and that includes the copy of overlays. > > Org-pending overlays work only in the buffer that owns the reglock: > so, to support indirect buffers, org-pending needs to detect and > remove such unwanted copies. Emacs should probably allow us to flag > our overlays as "not clonable into other buffers". Then, may you re-iterate what exactly is the problem if we do allow the overlays to be copied? Maybe we do not really need all the hassle with text properties? >>> (defun org-pending--user-cancel-default (reglock) >>> "Send a cancel message to REGLOCK to close it. >>> Default value for `org-pending-reglock-user-cancel-function'." >>> (org-pending-send-update >>> reglock (list :failure (list 'org-pending-user-cancel >>> "Canceled")))) >> >> What is the purpose of 'org-pending-user-cancel? > > `org-pending-user-cancel' is the error/condition symbol, defined using > 'define-error'. It's designed so that's easy to "unwrap" a message: > > (pcase outcome > (`(:failure ,err) (signal (car err) (cdr err))) > (`(:success ,v) v)) But I do not see any calls to `signal' in such context. Do I miss something? > Just in case, don't jump to a detailed code review of my changes in > ob-core yet, I've some planed changes there. Sure. That will be after we tidy up the core library. > Next set of comments about the org-pending library itself is very > welcome (as it's mostly independent of it's integration with org-babel). Here it is :) > (cl-defun org-pending--new-button-like-keymap (&key read-only) > "Return a new keymap for use on reglock overlays. > If READ-ONLY is non-nil, add bindings for read-only text else for > editable text." > (let ((map (make-sparse-keymap))) > (dolist (k `([touchscreen-down] [mouse-2] [mouse-1])) > (define-key map k #'org-pending-describe-reglock-at-point)) > (when read-only > (keymap-set map "RET" #'org-pending-describe-reglock-at-point)) > map)) > (defvar org-pending-outcome-keymap > (org-pending--new-button-like-keymap :read-only nil) > "Keymap for outcome overlays.") > (defvar org-pending-pending-keymap > (org-pending--new-button-like-keymap :read-only t) > "Keymap for pending region overlays.") Maybe we can make `org-pending-pending-keymap' inherit from `org-pending-outcome-keymap'? This way, if the latter is customized, the former will automatically update the bindings. > (defun org-pending--make-overlay (reglock type begin-end) > ... > (cl-flet ((make-read-only (ovl) > "Make the overly OVL read-only." > (overlay-put ovl 'modification-hooks read-only) > (overlay-put ovl 'insert-in-front-hooks read-only) > (overlay-put ovl 'insert-behind-hooks read-only))) You call `make-read-only' exactly once. cl-flet is redundant. > ... > (when (memq type '(:success :failure)) > (let ((bitmap (pcase type > (:success 'large-circle) > (:failure 'exclamation-mark))) > (face (pcase type > (:success 'org-done) > (:failure 'org-todo))) Bitmap and fringe face should be customizeable. > ( outcome nil > :documentation > "The outcome. nil when not known yet. Else a list: (:success RESULT) > or (:failure ERROR)") Nitpick: Here, and in a couple of nearby docstrings, there is a missing "."= at the very end > ( properties nil > :documentation > "A alist of properties. Useful to attach custom features to this REGLO= CK." ) Nitpick: Usually, "properties" imply plist. You might consider using a plist instead of alist or changing the field name to properties-alist. > ( -delete-outcome-marks (lambda ()) Can just use #'ignore as the default value. > (defun org-pending-reglock-useless-p (reglock) > "Return non-nil if REGLOCK is useless. > When a REGLOCK becomes useless, org-pending will, at some point, forget > about it." > (funcall (org-pending-reglock--useless-p reglock))) I feel like this idea of -useless-p is a kludge. We should use GC functionality instead, so that Emacs can take care about cleaning up things. Note that Emacs exposes custom GC handlers to Elisp level. Check out "2.4.20 Finalizer Type" section of Elisp manual. Here is a small demo: (setq bar 'non-nil) (with-temp-buffer (setq-local foo (list 1 (make-finalizer (lambda () (setq bar 'it-hapenned= )))))) # may need to do GC several times, until it actually decides to clean up th= e killed buffer (garbage-collect) (garbage-collect) bar > (cl-defun org-pending (region > &key anchor (name "REGLOCK") > (on-outcome #'org-pending-on-outcome-replace)) > ... > ... The default ON-OUTCOME > function replaces the region on success and ignores failures; in all > cases, it returns the outcome region (see the function > `org-pending-on-outcome-replace'). It is not clear what is used as the replacement by the default ON-OUTCOME. > You may set/update the following fields of your reglock to customize its > behavior: > - Emacs may have to destroy your locks; see the field > `before-destroy-function' if you wish to do something before your > lock is destroyed. It is not clear which "field" you are referring to when reading from top to bottom. Maybe, we can move "(cl-describe-type \\=3D'org-pending-reglock)" a bit earlier to avoid confusion. > You may add/update your own properties to your reglock using the field > `properties', which is an association list. I think that we should refer to `org-pending-reglock-property' here. > (let ((to-marker (lambda (p) > ;; Make sure P is a marker. > (or (and (markerp p) p) > (save-excursion (goto-char p) (point-marker))))) (copy-marker p) is shorter and does not involve moving point around. Also, we may want to copy these markers unconditionally, even when p is already the marker, it may have funny insertion type that slurps text inserted _after_ the marker (and locked region). So, maybe a simple unconditional (copy-marker p) is what we want. > (save-excursion > (setq anchor > (if (not anchor) > (let ((abeg ;; First non-blank point in region. The fact that ANCHOR does not include indentation is missing from the docstring. > (save-excursion (goto-char (car region)) > (re-search-forward "[[:blank:]]*") `skip-chars-forward' would be slightly faster, as we do not need match-data here. > (point-marker))) > (aend ;; Last position on first line > (save-excursion (goto-char (car region)) > (end-of-line) This will behave funny inside invisible text, because it honors point adjustments. Better use `line-end-position' + `copy-marker'. > (setq reglock (org-pending--make > :scheduled-at (float-time) > :-creation-point (point-marker) What is the purpose of -creation-point? It does not look like it is documented in `org-pending' docstring. Are there implicit assumptions about it in the other internal functions? > :-on-outcome on-outcome > ;; useless-p returns non-nil when a reglock becomes > ;; useless and we may forget about it. We'll update the > ;; function when we get the outcome. > :-useless-p (lambda () nil) #ignore > (defun org-pending-describe-reglock (reglock) > "Describe REGLOCK in a buffer. Maybe "Display a popup buffer describing REGLOCK."? > (one-line "Duration" > ;; TODO nice human format like 1m27s > (format "%.1fs" (org-pending-reglock-duration reglock))) `org-duration-from-minutes' > (defun org-pending-post-insert-outcome-default (lock message outcome-regi= on) > "Default value for `org-pending-post-insert-outcome-function'." > ;; We add some outcome decorations to let the user know what > ;; happened and allow him to explore the details. > (let* ((outcome-ovl (org-pending--make-overlay lock (car message) > outcome-region))) > (push `(apply delete-overlay ,outcome-ovl) buffer-undo-list) This will err when undo is disabled in buffer (and `buffer-undo-list' is set to t). > (cl-defun org-pending--update (reglock status data) > "Update REGLOCK to its new STATUS, using DATA. > Update the REGLOCK display to match the status STATUS (:scheduled, > :progress, :success, :failure). Also update the REGLOCK as needed. > Return nothing." > (cl-labels > ((add-style (status txt) > "Add the style matching STATUS over the text TXT." > (propertize txt 'face (org-pending-status-face status))) `add-style' is not used anywhere. > (short-version-of (msg) > "Compute the short version of MSG, to display on the anchor. > Must return a string." > (if msg > (car (split-string (format "%s" msg) "\n" :omit-nulls)) > "")) Maybe `truncate-string-to-width'? > (overlay-put anchor-ovl > 'before-string > ... > (propertize > (pcase status > (:scheduled "=E2=8F=B1") > (:pending "=E2=8F=B3") > (:failure "=E2=9D=8C") > (:success "=E2=9C=94=EF=B8=8F")) 1. indicator strings should be customizeable. 2. We need ASCII fallbacks of these symbols, if they cannot be displayed > (unless (and (consp outcome-region) > (or (integerp (car outcome-region)) > (markerp (car outcome-region))) > (or (integerp (cdr outcome-region)) > (markerp (cdr outcome-region)))) > (error "Not a region")))) `integer-or-marker-p' > (if (not outcome-region) > (setf (org-pending-reglock--useless-p reglock) > (lambda () t)) #'always > (let* ((pt (org-pending-reglock--creation-point reglock)) > (buf (marker-buffer pt))) > (message "org-pending: Handling update message at %s@%s: %s" > pt buf upd-message) > (save-excursion > (with-current-buffer buf > (save-excursion > (goto-char pt) It will be shorter to use `org-with-point-at' from org-macs.el Also, when marker is outside, narrowing, your version will behave unexpectedly. > (with-current-buffer buf > (save-excursion > (save-restriction > (widen) Or just (org-with-point-at start ...) > (if (> (- end start) 1) > ;; Insert in the middle as it's more robust to > ;; keep existing data (text properties, markers, > ;; overlays). > ... See `replace-region-contents' > (defun org-pending-locks-in (start end &optional owned) > "Return the list of locks in START..END. > Return the list of REGLOCK(s) that are alive between START and END, in > the current buffer. > When OWNED is non-nil, ignore locks that are not owned by this buffer. > See also `org-pending-locks-in-buffer-p'." > (let ((reglocks) > (here nil) > ovl) > (while (and (< start end) Mind the narrowing. > (defun org-pending--mgr-handle-reglock-update (reglock update) > "Handle the update UPDATE for this REGLOCK. > Return nothing." > (message "org-pending: reglock update for id=3D%s: %s" > (org-pending-reglock-id reglock) update)) Is it always desired to display a message each time a reglock is updated? --=20 Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at . Support Org development at , or support my work at