From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id UA7ROWgu5GK3EQEAbAwnHQ (envelope-from ) for ; Fri, 29 Jul 2022 21:00:57 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id kEXNOWgu5GJp+QAA9RJhRA (envelope-from ) for ; Fri, 29 Jul 2022 21:00:56 +0200 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 95F82E5BE for ; Fri, 29 Jul 2022 21:00:55 +0200 (CEST) Received: from localhost ([::1]:45320 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHVEE-0004eH-OZ for larch@yhetil.org; Fri, 29 Jul 2022 15:00:54 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37656) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHVDF-0004dN-53 for emacs-orgmode@gnu.org; Fri, 29 Jul 2022 14:59:53 -0400 Received: from mail.oa5.com ([89.187.75.252]:60826) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHVD8-0001qo-BR for emacs-orgmode@gnu.org; Fri, 29 Jul 2022 14:59:52 -0400 Received: from hugoLaptop (localhost.localdomain [127.0.0.1]) hugo@heagren.com (authenticated bits=0) by mail.oa5.com (8.14.4/8.12.11) with ESMTP id 26TIxWsY025177 sender hugo@heagren.com ; Fri, 29 Jul 2022 19:59:34 +0100 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=heagren.com; h= Message-ID:From:Subject:Date:To:MIME-Version:Content-Type; s= 202207; bh=UO8lP6PSgJ7myqxtWsIJLj5n2+Y=; b=V1GSPp6lL1d1kiCbwjGEh mC9C/OF0IqQiEKRHhftGTHvNvfsNPOq9tC7m0EYb7zCPccKadnLepod7s+26jMrt /7qTZEuswxAUyqbl1JUYpecjVz9CHXxJbd9Vevhx3H+JoSnQR8PSPGGHMnvtoQAy 6F/tZwIFdnKg7xl+4lutMIr6i2cM70atr7odbpJCwf6IL6mUub3MVYYO4CcdVH1Q BgW33lwgVoFTx8RK9K+hlklvDbQqQ/KQi45Ze4ZURbs X-Assp-Version: 2.6.7(22137) on ASSP.OA5.COM X-Assp-ID: ASSP.OA5.COM X-Assp-Session: EDE45C8 (mail 1) X-Assp-Envelope-From: hugo@heagren.com X-Assp-Intended-For: emacs-orgmode@gnu.org X-Assp-Intended-For: manikulin@gmail.com X-Assp-Client-TLS: yes Received: from global-5-155.n-2.net.cam.ac.uk ([131.111.5.155] helo=hugoLaptop) by ASSP.OA5.COM with SMTPSA(TLSv1_2 ECDHE-RSA-AES128-GCM-SHA256) (2.6.7); 29 Jul 2022 19:59:32 +0100 From: Hugo Heagren To: Max Nikulin Cc: emacs-orgmode@gnu.org Subject: Re: [PATCH v9] ol.el: add description format parameter to org-link-parameters References: <87zgl1npow.fsf@localhost> <20220405192931.6747-1-hugo@heagren.com> <87a6cx79xn.fsf@localhost> <87zgi6fckr.fsf@heagren.com> <871qvixhfw.fsf@gmail.com> <87v8s8n1bm.fsf@heagren.com> <87let39d3c.fsf@localhost> <877d4flu3x.fsf@heagren.com> <87cze5e84m.fsf@localhost> <87tu7gkb4l.fsf@heagren.com> <87y1ws6o0c.fsf@localhost> <87k08bjw0t.fsf@heagren.com> <47248a4f-10aa-0980-c054-563f30c05aaa@gmail.com> <87mtd0gthe.fsf@heagren.com> <78b97c9e-fced-0ee4-f3f2-3cbe81080ffa@gmail.com> <87sfms9dx7.fsf@localhost> <87v8rmd53g.fsf@localhost> <871qu9xv8q.fsf@heagren.com> <0da49392-26c6-8ba3-f657-647522d59342@gmail.com> Date: Fri, 29 Jul 2022 20:05:13 +0100 Message-ID: <87zggrg2om.fsf@heagren.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=89.187.75.252; envelope-from=hugo@heagren.com; helo=mail.oa5.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_PASS=-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" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1659121256; 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:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=Edt9CLI6bDpsbgMKtX726d5GfVvh5RFYWakgD89yxgY=; b=r9vw5vcO3/GyQ3hm952vV0MuzlSuD5lNZAMvWCVm6J8ZJLLXgaX/+N/Wu0C/z2yw6lQ3nE 1ijFWHOMJzvhmYO5HqZMPEQCnDzbuJhbF4dYo9U9H5pBzcdfjyVghnv4HHI/X8WLZDUWvz Hi7cpoDVH+ZtYO6Wm5toOfoRV8uHBC6jYeRLXQjPU1z/1egrRH4KqC7JEq/gaVZZT2L2kK 9iyOI2uEub3uAZQBPtrGN8F/3Zf4ufHnbJs33t7wp+d3fYFzJT0CBWa6CQ5ae9uhSzBVX5 5RoVMZJRPY0zwZciv/v91UY1dg0gcGlbw3dNz3l7EJtPyJiIRZIoLyL3W/t0bQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1659121256; a=rsa-sha256; cv=none; b=uZHzIoOjixC3I/HHTQapi6QG12a3NB2s9wNVEPinG29qG+nibaCPFZJ+zsJ+8gQt6FshVl gj1GBv+UcdCfQtO9ImBFRLJ/D75vFRXL9C1Ry8K3nDz3SQ3+vQOHD76qBf7kg19KeMJwOS wNCOP6fnvOV8Nlw4ysp0n6M6hLnGbEtBxm49p6uI2VfgOTfJVFG5i97WZciAE/46uAVcf9 p01AY9Yyukhelw9xxoddKLNbDTH1KOpyLplRCF4N9EHtedksrTd2Nj02DJUAuJ04aRv2BV znP1bYmy/6rhg2REx1AeArl2P6PGtzMfBP8jb0cZOhDMWODi8ZKG/YFP1tXRhg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=heagren.com header.s=202207 header.b=V1GSPp6l; dmarc=fail reason="SPF not aligned (relaxed)" header.from=heagren.com (policy=quarantine); 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" X-Migadu-Spam-Score: 6.00 X-Spam: Yes Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=heagren.com header.s=202207 header.b=V1GSPp6l; dmarc=fail reason="SPF not aligned (relaxed)" header.from=heagren.com (policy=quarantine); 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" X-Migadu-Queue-Id: 95F82E5BE X-Spam-Score: 6.00 X-Migadu-Spam: Yes X-Migadu-Scanner: scn0.migadu.com X-TUID: yqIsL8I1X6Sr --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > Hugo, almost certainly you are tired by so many iterations, but I > still can not approve your patch. No worries! I has to be right before it can be merged. I wouldn't want to submit faulty code. And besides, debugging today makes my code tomorrow better right? This was an interesting case. > signal(error ("rx form =E2=80=98or=E2=80=99 requires at least 1 args")) > apply(signal (error ("rx form =E2=80=98or=E2=80=99 requires at least 1 ar= gs"))) The test fails because of an error in `rx-to-string', which is called by `org-insert-link'. It was failing because I have the following code: ,---- | (rx-to-string `(: string-start (submatch (or ,@all-prefixes)) ":")) `---- > It is again Emacs-26 In the final test case, `all-prefixes' is nil at this point, and before Emacs 27.1, the rx form `or' /required/ arguments (it no longer does). So we need to fix the last case so that `or' has some arguments (`all-prefixes' is non-nil). The point of the last case is to test the behaviour when there is no relevant parameter for the link type. I /had/ done this by removing all the link parameters, but this makes `all-prefixes' nil, so we can't do that. We could just as easily do it by leaving the parameters as they are, and using a link 'type' which is definitely not in the list. I have taken this approach in the new version of the patch. I've used "fake-link-type", which will surely not be used, even in anyone's strange personal config. Admittedly it /could/ be used though (it would be possible to add it if someone wanted), so if you'd rather, I can develop something which uses a fake link type which is /by definition/ not in `org-link-parameters', it would just be rather a lot more work and the test case might subsequently be less clear to understand. Hope that helps -- do the tests pass for you now? Hugo --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename=0001-ol.el-add-description-format-parameter-to-org-link-p.patch Content-Transfer-Encoding: quoted-printable >From 039c2131462f5454f2804e809e085a055b5bf552 Mon Sep 17 00:00:00 2001 From: Hugo Heagren Date: Mon, 28 Mar 2022 23:18:45 +0100 Subject: [PATCH 1/2] ol.el: add description format parameter to org-link-parameters * ol.el (org-link-parameters): Add parameter `:insert-description', a string or a function. * (org-insert-link): If no description is provided (pre-existing or as an argument), next option is to use the `:insert-description' (if non-nil) parameter to generate one. * (org-link-make-description-function): Add documentation to describe behaviour of nil return value, like that of `:insert-description'. Default descriptions are predictable within a link type, but because link types are quite diverse, are NOT predictable across many types. A type-parameter is thus a good place to store information on the default description. --- etc/ORG-NEWS | 7 +++++ lisp/ol.el | 79 ++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 35af94f92..86128d100 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -322,6 +322,13 @@ purpose of the variable. The replacement variable accepts =3Dlistings=3D and =3Dverbatim=3D in place of =3Dt=3D and =3Dnil= =3D (which still work, but are no longer listed as valid options). =20 +*** ~org-link-parameters~ has a new ~:insert-description~ parameter + +The value of ~:insert-description~ is used as the initial input when +prompting for a link description. It can be a string (used as-is) or +a function (called with the same arguments as +~org-make-link-description-function~ to return a string to use). + * Version 9.5 =20 ** Important announcements and breaking changes diff --git a/lisp/ol.el b/lisp/ol.el index d4bd0e40c..abea2af5e 100644 --- a/lisp/ol.el +++ b/lisp/ol.el @@ -141,6 +141,19 @@ link. Function that inserts a link with completion. The function takes one optional prefix argument. =20 +`:insert-description' + + String or function used as a default when prompting users for a + link's description. A string is used as-is, a function is + called with two arguments: the link location (a string such as + \"~/foobar\", \"id:some-org-id\" or \"https://www.foo.com\") + and the description generated by `org-insert-link'. It should + return the description to use (this reflects the behaviour of + `org-link-make-description-function'). If it returns nil, no + default description is used, but no error is thrown (from the + user's perspective, this is equivalent to a default description + of \"\"). + `:display' =20 Value for `invisible' text property on the hidden parts of the @@ -200,7 +213,9 @@ You can interactively set the value of this variable by= calling This function must take two parameters: the first one is the link, the second one is the description generated by `org-insert-link'. The function should return the description to -use." +use. If it returns nil, no default description is used, but no +error is thrown (from the user=E2=80=99s perspective, this is equivalent +to a default description of \"\")." :group 'org-link :type '(choice (const nil) (function)) :safe #'null) @@ -1804,11 +1819,14 @@ prefix negates `org-link-keep-stored-after-insertio= n'. If the LINK-LOCATION parameter is non-nil, this value will be used as the link location instead of reading one interactively. =20 -If the DESCRIPTION parameter is non-nil, this value will be used as the -default description. Otherwise, if `org-link-make-description-function' -is non-nil, this function will be called with the link target, and the -result will be the default link description. When called non-interactivel= y, -don't allow to edit the default description." +If the DESCRIPTION parameter is non-nil, this value will be used +as the default description. If not, and the chosen link type has +a non-nil `:insert-description' parameter, that is used to +generate a description as described in `org-link-parameters' +docstring. Otherwise, if `org-link-make-description-function' is +non-nil, this function will be called with the link target, and +the result will be the default link description. When called +non-interactively, don't allow to edit the default description." (interactive "P") (let* ((wcf (current-window-configuration)) (origbuf (current-buffer)) @@ -1818,7 +1836,10 @@ don't allow to edit the default description." (desc region) (link link-location) (abbrevs org-link-abbrev-alist-local) - entry all-prefixes auto-desc) + (all-prefixes (append (mapcar #'car abbrevs) + (mapcar #'car org-link-abbrev-alist) + (org-link-types))) + entry auto-desc) (cond (link-location) ; specified by arg, just use it. ((org-in-regexp org-link-bracket-re 1) @@ -1859,9 +1880,6 @@ Use TAB to complete link prefixes, then RET for type-= specific completion support (unless (pos-visible-in-window-p (point-max)) (org-fit-window-to-buffer)) (and (window-live-p cw) (select-window cw)))) - (setq all-prefixes (append (mapcar #'car abbrevs) - (mapcar #'car org-link-abbrev-alist) - (org-link-types))) (unwind-protect ;; Fake a link history, containing the stored links. (let ((org-link--history @@ -1958,17 +1976,36 @@ Use TAB to complete link prefixes, then RET for typ= e-specific completion support (setq desc path))))) =20 (unless auto-desc - (let ((initial-input - (cond - (description) - ((not org-link-make-description-function) desc) - (t (condition-case nil - (funcall org-link-make-description-function link desc) - (error - (message "Can't get link description from %S" - (symbol-name org-link-make-description-function)) - (sit-for 2) - nil)))))) + (let* ((type + (cond + ((string-match (rx-to-string `(: string-start (submatch (or= ,@all-prefixes)) ":")) link) + (match-string 1 link)) + ((file-name-absolute-p link) "file") + ((string-match "\\`\\.\\.?/" link) "file"))) + (initial-input + (cond + (description) + (desc) + ((org-link-get-parameter type :insert-description) + (let ((def (org-link-get-parameter type :insert-descriptio= n))) + (condition-case nil + (cond + ((stringp def) def) + ((functionp def) + (funcall def link desc))) + (error + (message "Can't get link description from org link pa= rameter `:insert-description': %S" + def) + (sit-for 2) + nil)))) + (org-link-make-description-function + (condition-case nil + (funcall org-link-make-description-function link desc) + (error + (message "Can't get link description from %S" + org-link-make-description-function) + (sit-for 2) + nil)))))) (setq desc (if (called-interactively-p 'any) (read-string "Description: " initial-input) initial-input)))) --=20 2.20.1 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-test-ol-tests-for-insert-description-param-when-inse.patch >From 708916f8d4f31af1e786e87154b5a4479d0ed1b8 Mon Sep 17 00:00:00 2001 From: Hugo Heagren Date: Sat, 16 Jul 2022 19:50:15 +0100 Subject: [PATCH 2/2] test-ol: tests for insert-description param when inserting links * test-ol (test-ol-with-link-parameters-as): Convenience macro for testing. (test-ol-insert-link-get-desc): Convenience macro for testing. (test-ol/return-foobar): Convenience function for testing. (test-ol/insert-link-insert-description): Test for various values of `:insert-description' in `org-link-parameters' (including `test-ol/return-foobar'). --- testing/lisp/test-ol.el | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el index 429bb52ee..ac01c21eb 100644 --- a/testing/lisp/test-ol.el +++ b/testing/lisp/test-ol.el @@ -19,6 +19,8 @@ ;;; Code: +(require 'cl-lib) + ;;; Decode and Encode Links @@ -625,5 +627,96 @@ See https://github.com/yantar92/org/issues/4." (test-ol-parse-link-in-text "The http://foo.com/(something)?after=parens link")))) +;;; Insert Links + +(defmacro test-ol-with-link-parameters-as (type parameters &rest body) + "Pass TYPE/PARAMETERS to `org-link-parameters' and execute BODY. + +Save the original value of `org-link-parameters', execute +`org-link-set-parameters' with the relevant args, execute BODY +and restore `org-link-parameters'. + +TYPE is as in `org-link-set-parameters'. PARAMETERS is a plist to +be passed to `org-link-set-parameters'." + (declare (indent 2)) + (let (orig-parameters) + ;; Copy all keys in `parameters' and their original values to + ;; `orig-parameters'. + (cl-loop for param in parameters by 'cddr + do (setq orig-parameters + (plist-put orig-parameters param (org-link-get-parameter type param)))) + `(unwind-protect + ;; Set `parameters' values and execute body. + (progn (org-link-set-parameters ,type ,@parameters) ,@body) + ;; Restore original values. + (apply 'org-link-set-parameters ,type ',orig-parameters)))) + +(defun test-ol-insert-link-get-desc (&optional link-location description) + "Insert link in temp buffer, return description. + +LINK-LOCATION and DESCRIPTION are passed to +`org-insert-link' (COMPLETE-FILE is always nil)." + (org-test-with-temp-text "" + (org-insert-link nil link-location description) + (save-match-data + (when (and + (org-in-regexp org-link-bracket-re 1) + (match-end 2)) + (match-string-no-properties 2))))) + +(defun test-ol/return-foobar (_link-test _desc) + "Return string \"foobar\". + +Take (and ignore) arguments conforming to `:insert-description' +API in `org-link-parameters'. Used in test +`test-ol/insert-link-insert-description', for the case where +`:insert-description' is a function symbol." + "foobar-from-function") + +(ert-deftest test-ol/insert-link-insert-description () + "Test `:insert-description' parameter handling." + ;; String case. + (should + (string= + "foobar-string" + (test-ol-with-link-parameters-as + "id" (:insert-description "foobar-string") + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; Lambda case. + (should + (string= + "foobar-lambda" + (test-ol-with-link-parameters-as + "id" (:insert-description (lambda (_link-test _desc) "foobar-lambda")) + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; Function symbol case. + (should + (string= + "foobar-from-function" + (test-ol-with-link-parameters-as + "id" (:insert-description #'test-ol/return-foobar) + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; `:insert-description' parameter is defined, but doesn't return a + ;; string. + (should + (null + (test-ol-with-link-parameters-as + "id" (:insert-description #'ignore) + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; Description argument should override `:insert-description'. + (should + (string= + "foobar-desc-arg" + (test-ol-with-link-parameters-as + "id" (:insert-description "foobar") + (test-ol-insert-link-get-desc "id:foo-bar" "foobar-desc-arg")))) + ;; When neither `:insert-description' nor + ;; `org-link-make-description-function' is defined, there should be + ;; no description + (should + (null + (let ((org-link-make-description-function nil)) + (test-ol-insert-link-get-desc "fake-link-type:foo-bar"))))) + (provide 'test-ol) ;;; test-ol.el ends here -- 2.20.1 --=-=-=--