From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id 4EWIAWH6qWYnTwAAe85BDQ:P1 (envelope-from ) for ; Wed, 31 Jul 2024 08:48:33 +0000 Received: from aspmx1.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id 4EWIAWH6qWYnTwAAe85BDQ (envelope-from ) for ; Wed, 31 Jul 2024 10:48:33 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Yme4lfqL; 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=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1722415712; 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=cEuqJYDQcatZYgEcalj7fEBOZr2L0zCZbj/KOXYq32M=; b=NcXlJ2PVuyS7LSvTA16lq0Exar0SJ/R1jxR6Y/w1Lbt4BrP3Cmaf8qNGapU2v5Pr1AuaPg XdQ83ND3MpxsvAfMqSSe0h1Z1IUrl1HxLia7Aj93Dp/BXrE0sdiABD9RgEtHeEiauHqowq wbxd5bCdf8Fv80xswesqlDv9GumTlAZnWu8M/etJoacZYKyKIn3K6aV+JGnMOe9X6vwEX5 VNCPg/fY7vRvBy3fIq4bU7/Noe2WSQLSSaarw3Iazwu+r3LSSs4K+b5LvWSn/Y8tPrwfj2 +PYd/fly7EK6LFTFpNQcfFa89WuY7o2qOeGS8AQnG/GnekmnPZ/aFv1OnXeQpA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Yme4lfqL; 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=gmail.com ARC-Seal: i=1; s=key1; d=yhetil.org; t=1722415712; a=rsa-sha256; cv=none; b=YysNJ7ir3BQI3XCSznV8i8mzeO6jjr9ribJFFrdrMpnPqCC1QozEnWcL/gLW+pLBfEuPhi iUHYsrnHbkE3dgs9bYeuUrX83Et1OdHWymL8885/67ZY3v/rWfJRQpnFivu0wMWBuN4i22 DYS8lun3W7dlymNILwcTo4Oc76mprsiz+uhy52GYmtJvG0AO4OBq1LKTjx5+bbiyGiQPGj fWHUCH8Zu5GhNeGpwXgja91hLjlc9TvXUpsnWnr3te0/4dDRvMqtxrK8tpvWn3qyz3/AWa SQZp3dszR3uc+/efJcH35IUOxitKQ6kmQV56mHcSYSXrSU5SvaI4jnyiVg0hMw== 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 E69D162DA6 for ; Wed, 31 Jul 2024 10:48:31 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sZ4zg-0001gb-Rs; Wed, 31 Jul 2024 04:47:36 -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 1sZ4ze-0001g8-NS for emacs-orgmode@gnu.org; Wed, 31 Jul 2024 04:47:35 -0400 Received: from mail-wr1-x431.google.com ([2a00:1450:4864:20::431]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sZ4zc-0004Ds-F5 for emacs-orgmode@gnu.org; Wed, 31 Jul 2024 04:47:34 -0400 Received: by mail-wr1-x431.google.com with SMTP id ffacd0b85a97d-368440b073bso449946f8f.0 for ; Wed, 31 Jul 2024 01:47:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722415650; x=1723020450; darn=gnu.org; h=mime-version:date:references:in-reply-to:subject:cc:to:from :message-id:from:to:cc:subject:date:message-id:reply-to; bh=cEuqJYDQcatZYgEcalj7fEBOZr2L0zCZbj/KOXYq32M=; b=Yme4lfqLTRLWpGtazrpmW+iAg04gxU76QEwAvYns3I7kzCmGGZHT+89XLqWvrBU8xj 6J5hWVv7cZJTArKxUwGTH+moe6JHgk1HwO3CH1vrncKssAszajDgKnAZn1adEzo30ic1 nV+m1pnYoSLLGq+bDNlX4kwQGET5v4esW9qm6Sq6DJuy01xFftzm1YM6tRguJuQgXYK1 GTa+xOPzwFe3gGn56k4+6xmclCq25pHTTJYox6HwRy1HFD/cQiEJFO/s5A/wT8Jy4zLB m9FcmIYpoAShKJ1SUd7awaH9HJEj+0xNJlKb1CofNgetT8LH73/gTcMj+4C+PYYlQScY +opQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722415650; x=1723020450; h=mime-version:date:references:in-reply-to:subject:cc:to:from :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cEuqJYDQcatZYgEcalj7fEBOZr2L0zCZbj/KOXYq32M=; b=ix/7C8Ys4rLKgpgkDl6j8G1jH7/+KghfPNVtk6QiQO5mm02dib6t8vB0K+gtM4vvCL s4nRV9EEovGhixiH1b4Kxvlh56baQKwFApLUGQU0bJssD6JOVhwBbhTKphBeTNGAheXG cyNmboha9wCDy06peueOarpDA3AQG3W6ZXUf/eE80iRfRZpwfrTPpZYPW+uaoak96O8E fmJqMEtNJ09O0Wf++GwM1EaflO0EDFPo5gjyq6sdQY1x6wA5JbtnxrRxo0kq1kbM/QQt 6BUdXgFtgKVDnog9fB77/Va7jsJjcu2bG4C/TCnrJSCLd9JvrrJj54O1f3nefLNMRaf0 ZlZA== X-Gm-Message-State: AOJu0YyRubjKFSiXC6+MZG6qQfa1ar7dgMLAkIAVjQVqewZxj4ptn8RL mcPlWhLe/84Y1ix5oAMK1nzcDDPon6PN0J+uPxdbrB9K1qXuT92m X-Google-Smtp-Source: AGHT+IGcdyzcJrUfwbq9tMJuATsV1iW5/1ZM4jgZTTnQ3XBhJ+LunsbwWWwSMmQ7V3lChch7LEVz6g== X-Received: by 2002:a05:6000:ccc:b0:367:8a2d:b354 with SMTP id ffacd0b85a97d-36b8c8df447mr3134209f8f.14.1722415650090; Wed, 31 Jul 2024 01:47:30 -0700 (PDT) Received: from localhost ([2a01:e0a:505:3460:1c18:688d:ece4:372e]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36b3686227csm16503219f8f.99.2024.07.31.01.47.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 01:47:29 -0700 (PDT) Message-ID: <66a9fa21.5d0a0220.624f7.d3d6@mx.google.com> Received: by localhost (sSMTP sendmail emulation); Wed, 31 Jul 2024 10:47:27 +0200 From: Bruno Barbier To: Ihor Radchenko Cc: emacs-orgmode , Jack Kamm , Matt Subject: Re: Pending contents in org documents In-Reply-To: <87y15x4g11.fsf@localhost> References: <87o7d0mm54.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> <87y15x4g11.fsf@localhost> Date: Wed, 31 Jul 2024 10:47:27 +0200 MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=2a00:1450:4864:20::431; envelope-from=brubar.cs@gmail.com; helo=mail-wr1-x431.google.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, FREEMAIL_FROM=0.001, MSGID_FROM_MTA_HEADER=0.001, RCVD_IN_DNSWL_NONE=-0.0001, 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-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Queue-Id: E69D162DA6 X-Migadu-Scanner: mx13.migadu.com X-Migadu-Spam-Score: -9.75 X-Spam-Score: -9.75 X-TUID: 3c6ySUJiuBRf Hi Ihor, Ihor Radchenko writes: > 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? > The function 'org-pending-unlock-NOW!' is part of the API, it's not a command Emacs end users. If we make it a command and display that button by default, we'll need also an option to not display it by default, and, probably an other option to not ask for confirmation. Let see later if we really need to provide all this. >>> 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. In my view, reglocks are more about regions that are being updated, than processes; they are kind of _planed_ buffer modifications. But, anyway, I added a user option `org-pending-confirm-ignore-reglocks-on-exit' that allows to ignore pending locks when exiting. >>> 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 > Thanks. >> (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. They were derived from built-in ones (error, success, org-tag, etc.). I've redesigned the faces and put them all in org-pending, so that org-pending is indenpendent of Org. I'm now computing some colours from built-in faces to avoid colour names. I'm not sure it's what you meant, as even Org itself doesn't do this. >> :version "30.1" > Please, use :package-version instead of :version. I'm now using: :package-version '(Org . "9.7") >> (defun org-pending--status-still-pending-p (status) > > This function is trivial and used only once. Maybe you can just inline > it. Right. But it also defines what "still pending" means. I prefer to keep 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? They are now part of the public API. > It may also be a good idea to add some actual keyboard binding to this > map. Right. I've improved the keymaps, trying to work like a button when the text is read-only. >> (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. Indeed. I moved it into ob-core, where it is used. >> (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. > Done. Thanks. >> (defun org-pending-reglock-duration (reglock) > > The docstring does not mention REGLOCK argument. Done. >> (defun org-pending-reglock-property (reglock prop) > > ... PROP argument. Done. >> (defun org-pending-reglock-set-property (reglock prop val) >> "See `org-pending-reglock-property'." > > May as well write the full docstring. Done. >> (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. Oops. Sorry. > >> (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) Done. Thanks. > >> (user-error "This region lock has been destroyed.")))) > > Error messages should not end with period. Oops. Sorry. >> (let ((buffer (get-buffer-create "*Region Lock*"))) > > Please avoid using constant strings. Instead, declare them as defvar. Done. >> (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. Thanks. And I've fixed the doc to more accurately describe what's happening. > May users want to see info about multiple reglocks at the same time? The function `org-pending-describe-reglock' works like `describe-variable' and other Emacs related functions: the user needs to rename the buffer manually if he wants to keep it. >> (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) Done. Thanks for the references to the command and to one example. >> (defun org-pending--describe-reglock-at-point () > > Why internal? It is a command users may want to call via M-x. OK. >> (defun org-pending-pre-insert-outcome-default (_lock _message) >> "Default value for `org-pending-pre-insert-outcome-function'.") > > May as well just use #'ignore OK. Done. > >> (defvar org-pending-pre-insert-outcome-function >> (defvar org-pending-post-insert-outcome-function > > Should these be defcustom ? No. This is part of the developer API, not for the end user. > >> (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'). Done. >> (cl-defun org-pending--update (reglock status data) > > No docstring. Please, add. I added some basic documentation; I hope this is enough (this is an internal function). > >> (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? Good point. I'm now using an inlined without-restriction. >> (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. Done. I've inlined its definition. I've pushed the changes to my branch. Thanks again for your review and time, Bruno