From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id 6JKPO2W3sWLtXgEAbAwnHQ (envelope-from ) for ; Tue, 21 Jun 2022 14:19:50 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id WDp4O2W3sWIBaQAAauVa8A (envelope-from ) for ; Tue, 21 Jun 2022 14:19:49 +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 94EA2BEC0 for ; Tue, 21 Jun 2022 14:19:48 +0200 (CEST) Received: from localhost ([::1]:52044 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o3crD-0006bb-Ou for larch@yhetil.org; Tue, 21 Jun 2022 08:19:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47012) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o3cq8-0005cT-3z for emacs-orgmode@gnu.org; Tue, 21 Jun 2022 08:18:41 -0400 Received: from [2a01:c0:2:6:225:90ff:fed2:2b47] (port=54081 helo=mail.oa5.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o3cq3-0007Ne-Kc for emacs-orgmode@gnu.org; Tue, 21 Jun 2022 08:18:39 -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 25LBwNZ1021178 sender hugo@heagren.com for ; Tue, 21 Jun 2022 12:58:24 +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= 202206; bh=Q99sTvocQISx90HxdRpwCqsdXDI=; b=xIjq+XtJe86xbhxMSROpO 0NCqD9ktYiv9jpUYQT6TG44Hgt6zcPU5MJs+jKBWxD1HXmgFbzO0i7c6VwUhEz2b JTBcFDUsLiT9PerFq+e8mmdgfvjImGKZhqq6w8b3eyohSFWzpktw0D4dngXuY8CX y6sR7Du8g72rAeNRezIvN1xgqmYH6CTdAV5kFbIZoTbR5VMHsm8h/bMDDu0fGDrq NSqVomJNwQnvq1mPp6RP9MYhRbM445dEWYJy/oTrijR X-Assp-Version: 2.6.7(22137) on ASSP.OA5.COM X-Assp-ID: ASSP.OA5.COM X-Assp-Session: 7FE23C2E46B0 (mail 1) X-Assp-Envelope-From: hugo@heagren.com X-Assp-Intended-For: emacs-orgmode@gnu.org X-Assp-Client-TLS: yes Received: from global-5-154.nat-2.net.cam.ac.uk ([131.111.5.154] helo=hugoLaptop) by ASSP.OA5.COM with SMTPSA(TLSv1_2 ECDHE-RSA-AES128-GCM-SHA256) (2.6.7); 21 Jun 2022 12:58:23 +0100 From: Hugo Heagren To: emacs-orgmode@gnu.org Subject: Re: [PATCH v3] ol.el: add description format parameter to org-link-parameters References: <87zgl1npow.fsf@localhost> <20220405192931.6747-1-hugo@heagren.com> <87a6cx79xn.fsf@localhost> Date: Tue, 21 Jun 2022 13:03:32 +0100 Message-ID: <87zgi6fckr.fsf@heagren.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Host-Lookup-Failed: Reverse DNS lookup failed for 2a01:c0:2:6:225:90ff:fed2:2b47 (failed) Received-SPF: pass client-ip=2a01:c0:2:6:225:90ff:fed2:2b47; envelope-from=hugo@heagren.com; helo=mail.oa5.com X-Spam_score_int: -12 X-Spam_score: -1.3 X-Spam_bar: - X-Spam_report: (-1.3 / 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, RDNS_NONE=0.793, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no 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=1655813989; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=sUPIuKyQLIQNeJD4JSFFmP5mAtNsy57fXQvVN47XYGg=; b=emmpLbwqWduwp/mxYEsEVT6j/Q31IkmlEft26RYfvZ5EG4NTIoXmyOiVahhKJw6ExYUhAT +zlCvVPJQWlMMsXypZarfjOyTq7dYbpXm5mJJgn825YtzhDzkGBj1HAVbsl7jClJErespD Og7aG/4lrHvgRyLAv0hHbrSmk3AusUO3oP89O+fnXttQtM3qQ+rddcRJKMkM0Q3sL6fKS7 CCVY/HTTLO9Kugo2hFxM0scN5vO5EZote3/Lxipo/4hugmeOYYWiWmJywBftmYBaxJIRwN iW3ZmOejuodUGu1kJFOAvEP2HkMBJDeNv+8NHxoPzpKoV00LSZbFa902WCA1lQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1655813989; a=rsa-sha256; cv=none; b=UTwKrfuVYmiiHMgCswxpeQKX6X8PLoQV7yjISNjiiVU+cibRwgUSXSdYw0oVGD0GSpEwNt iAkrRHCWyS4ouwkvMw+cmgctfJ/FqN86vl1SPby0NSdnqE+k6l2BpXxqFuCJgU1I+lwE9N fro0YReFfN6Q6ZUo1AnYJihEKWMm8skCY/VHsGzBqXvyZ5aFJ7FzuX8XzSqzYRgHDbf430 CFp/c/YM06XxdwpCf/PYQPKAZtYWDC4uzcYCERfM8sjXOJ2J0Fyk3gFS+xbzT1T868bMkv sTcWfS9lNFCcdU/Y1LdXkxrNCf7/Vw5Dpaor0djCARl5lH4h+CaxWb0xzaE2gA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=heagren.com header.s=202206 header.b=xIjq+XtJ; dmarc=pass (policy=quarantine) header.from=heagren.com; 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: -5.47 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=heagren.com header.s=202206 header.b=xIjq+XtJ; dmarc=pass (policy=quarantine) header.from=heagren.com; 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: 94EA2BEC0 X-Spam-Score: -5.47 X-Migadu-Scanner: scn0.migadu.com X-TUID: Lbs3y5Tw4Gl3 --=-=-= Content-Type: text/plain Content-Disposition: inline I've finally had time to the `:default-description' parameter in `org-link-parameters'. Ihor Radchenko writes: > I recommend writing tests for org-insert-link in > testing/lisp/test-ol.el and testing expected behaviour for various > values of :default-description. Good advice! I haven't written many tests before, so this was a challenge. I've attached a patch with some tests for the expected behaviour. These include a couple of macros and a function, to stop the definitions being too huge and unwieldy. All the tests pass on my machine. They're very nearly just right, but for some reason `test-ol-with-link-parameters-as' doesn't always reset the parameters correctly for me. However I have them set to originally, it sets `:default-description' to a lambda. This might be a quirk at my end though -- if someone else could have a look I'd be very grateful. The tests showed that my previous approach doesn't work generally enough. I've moved where `type' is defined in the code, to cover all cases. > Firstly, def will be undefined inside the error clause. I've defined def a bit further up in a `let', and run the error clause inside this. > Secondly, when :default-description is a lambda expression and that > expression fails, your code will fail with "wrong-type-argument > symbolp" when executing (symbol-name def). Please consider cases > when `def' is not a string and not a function symbol. Dealt with this by just using `message " %S: "', since %S will print a symbol, a string or a lambda correctly. Hope this is a bit better! Best, Hugo --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-test-ol-tests-for-default-description-param-when-ins.patch >From 5a44edfda4e09a95bba1327c2758b2637e5b16bd Mon Sep 17 00:00:00 2001 From: Hugo Heagren Date: Tue, 21 Jun 2022 12:45:50 +0100 Subject: [PATCH 2/2] test-ol: tests for default-description param when inserting links Add tests for various values of `:default-description' in `org-link-parameters'. --- testing/lisp/test-ol.el | 77 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el index 429bb52ee..7524d73af 100644 --- a/testing/lisp/test-ol.el +++ b/testing/lisp/test-ol.el @@ -625,5 +625,82 @@ 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'." + `(progn + (setq orig-parameters org-link-parameters) + (org-link-set-parameters ,type ,@parameters) + (let ((rtn (progn ,@body))) + (setq org-link-parameters orig-parameters) + rtn))) + +(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 `:default-description' +API in `org-link-parameters'. Used in test +`test-ol/insert-link-default-description', for the case where +`:default-description' is a function symbol." + "foobar") + +(ert-deftest test-ol/insert-link-default-description () + "Test `:default-description' parameter handling." + ;; String case + (should + (string= + "foobar" + (test-ol-with-link-parameters-as + "id" (:default-description "foobar") + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; Lambda case + (should + (string= + "foobar" + (test-ol-with-link-parameters-as + "id" (:default-description (lambda (_link-test _desc) "foobar")) + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; Function symbol case + (should + (string= + "foobar" + (test-ol-with-link-parameters-as + "id" (:default-description #'test-ol/return-foobar) + (test-ol-insert-link-get-desc "id:foo-bar")))) + ;; `:default-description' parameter is defined, but doesn't return a + ;; string + (should-error + (test-ol-with-link-parameters-as + "id" (:default-description #'ignore) + (test-ol-insert-link-get-desc "id:foo-bar"))) + ;; Description argument should override `:default-description' + (should + (string= + "notfoobar" + (test-ol-with-link-parameters-as + "id" (:default-description "foobar") + (test-ol-insert-link-get-desc "id:foo-bar" "notfoobar"))))) + (provide 'test-ol) ;;; test-ol.el ends here -- 2.20.1 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-ol.el-add-description-format-parameter-to-org-link-p.patch >From 437155c2394b5c9961ecc0af4e1e1a54b63416fe 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 `:default-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 `:default-description' (if non-nil) parameter to generate one. 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. --- lisp/ol.el | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/lisp/ol.el b/lisp/ol.el index d4bd0e40c..1ab3a5f76 100644 --- a/lisp/ol.el +++ b/lisp/ol.el @@ -141,6 +141,15 @@ link. Function that inserts a link with completion. The function takes one optional prefix argument. +`:default-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 full link text, 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'). + `:display' Value for `invisible' text property on the hidden parts of the @@ -1804,11 +1813,14 @@ prefix negates `org-link-keep-stored-after-insertion'. If the LINK-LOCATION parameter is non-nil, this value will be used as the link location instead of reading one interactively. -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-interactively, -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 `:default-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 +1830,7 @@ don't allow to edit the default description." (desc region) (link link-location) (abbrevs org-link-abbrev-alist-local) - entry all-prefixes auto-desc) + entry all-prefixes auto-desc type) (cond (link-location) ; specified by arg, just use it. ((org-in-regexp org-link-bracket-re 1) @@ -1893,6 +1905,13 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (or entry (push link org-link--insert-history)) (setq desc (or desc (nth 1 entry))))) + (setq type + (cond + ((string-match (rx (: string-start (submatch (eval `(or ,@all-prefixes))) ":")) link) + (match-string 1 link)) + ((file-name-absolute-p link) "file") + ((string-match "\\`\\.\\.?/" link) "file"))) + (when (funcall (if (equal complete-file '(64)) 'not 'identity) (not org-link-keep-stored-after-insertion)) (setq org-stored-links (delq (assoc link org-stored-links) @@ -1961,12 +1980,24 @@ Use TAB to complete link prefixes, then RET for type-specific completion support (let ((initial-input (cond (description) - ((not org-link-make-description-function) desc) + (desc) + ((org-link-get-parameter type :default-description) + (let ((def (org-link-get-parameter type :default-description))) + (condition-case nil + (cond + ((stringp def) def) + ((functionp def) + (funcall def link desc))) + (error + (message "Can't get link description from org link parameter `:default-description': %S" + def) + (sit-for 2) + nil)))) (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)) + org-link-make-description-function) (sit-for 2) nil)))))) (setq desc (if (called-interactively-p 'any) -- 2.20.1 --=-=-=--