From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0.migadu.com ([2001:41d0:303:5f26::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms8.migadu.com with LMTPS id yE6bFutoe2XTFQAAkFu2QA (envelope-from ) for ; Thu, 14 Dec 2023 21:43:23 +0100 Received: from aspmx1.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0.migadu.com with LMTPS id MCY/EOtoe2XQpwAAqHPOHw (envelope-from ) for ; Thu, 14 Dec 2023 21:43:23 +0100 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=ricklupton.name header.s=fm3 header.b="3 Aoab+J"; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=zt+9X1Zu; 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=ricklupton.name ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1702586603; 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=wgYPp1punAmujgjpRX+QHHBB5ucFtigC/03m1THh+ls=; b=o/MHUZaeBnk5//BFvYon2Ob/kGZLGjur8ZeTS47b3ugHw/5QztH2zdO9FVLNmcy8pNSM9L LOiGj8xw7rXXIo44VwF+Pw6QkUzF2KH/xinObJ1wEEXFjIizH0lHvrUXl5CRJOQVUtYKGi 0wgMUEJmVUOKsL+pA0BMZx5va/5iJyhXWOpH/P0kzDwmIWfT0ow3p0hh6t76+z6FodUBtf 3oGzgpo2VXQ0gL43j+H+6qL5F6YbVME6oV0IvrFQUw5yyrGgqHyUQGm2yFleT6VRkscbXN cYKJjh2rj+1bQQmf7yr2d/dvmHSZsu+oB65z8NCItiJSL7Nlj7L/wSsERknPMw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=ricklupton.name header.s=fm3 header.b="3 Aoab+J"; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=zt+9X1Zu; 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=ricklupton.name ARC-Seal: i=1; s=key1; d=yhetil.org; t=1702586603; a=rsa-sha256; cv=none; b=OZ/Se6NHt4GIFGz8ecr9EDLHLoFTP1t5qCju2BVeHwAQtYr4VWTaYBRcPnNCkE4veTmju5 MyQ1FhlFfE+MtkGwbEBJVq961lnt4Vsh2EzzgwrvCpexYTwxcupyXiPmeWt8NC7wqjxt5L DWsDlU6WVRjA03d6SDEB6LCq82IgBN2+8RZRkHGDomW228CFs1YHzkH1E8VMuUiydx6eWU oldpwPTNbH3oVmoyaKRpPXkTd3Sfkvxse5eQBgeqsg5/hL6WB/vx+/dUpR01SQdYW5c/Qw JGBNQCQcvkesErYNGdFN1ZvBYBSGOwOtt3f/1OYQaAQeF+sQEYvnw4y+ulMRmw== 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 3442332752 for ; Thu, 14 Dec 2023 21:43:22 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rDsXP-0004Ib-Eu; Thu, 14 Dec 2023 15:42:31 -0500 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 1rDsXN-0004IN-Ao for emacs-orgmode@gnu.org; Thu, 14 Dec 2023 15:42:29 -0500 Received: from out3-smtp.messagingengine.com ([66.111.4.27]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rDsXL-0001aL-1T for emacs-orgmode@gnu.org; Thu, 14 Dec 2023 15:42:29 -0500 Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 362C55C015B; Thu, 14 Dec 2023 15:42:25 -0500 (EST) Received: from imap50 ([10.202.2.100]) by compute2.internal (MEProxy); Thu, 14 Dec 2023 15:42:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ricklupton.name; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm3; t=1702586545; x= 1702672945; bh=wgYPp1punAmujgjpRX+QHHBB5ucFtigC/03m1THh+ls=; b=3 Aoab+J/3QwtTfx+kYPxbtlnQKoGPzciJlxgfJo5eoJbU8PvhXYXapV0jSe/hcFix g/egvhfDESBx3VVGydNEvZrgbz0YBpxO209cYPolUKLNd4n0qizFNIMlxC0EJ+8J yhtYKNh9FSW+S0rrvsGKedjykze/8RLq1g7kBNYQ+q6NWHO4DIW7Ecw+s41tL0Ss vdXlh1HAwFRZWVujCEENlXYf/3Fe+t7IfeBlMlLffwF3Ax9PDGT/al33awSx2HT/ OklWtYlLjIHi9gCESFMfzTwPL1Mv55gG8cIIvMqyzi0NSkadH8oaVhFq1SEtgZAh IbSSUVyvQWTMnwX3pHj6w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1702586545; x=1702672945; bh=wgYPp1punAmujgjpRX+QHHBB5ucF tigC/03m1THh+ls=; b=zt+9X1ZuSnWJeMKOECrtaKOr6LVX5qLxgHKOnyp+oxA0 nhI6k7xzAbSjev8Nm2VdGbS4wiHw2zoeBLeLYwdLd1Y7qxXlGStuaYNAxgjVZp8L zPZcHmuSxh/aU/FmAKnuFf0yi5QAoltVuMKXDd7aL7pPj/PiExCsXSr9qr+BXzl6 vSly+hidifLggrkNW+ZShjg5CKyYFNWX1cNCfbZc2OrEWcLhSVazHe2w7OYpOQed /gdHdWtqb3cVTK/VI26nalCWXSzs1g61wJ6N3bxz5pVcz6Nx1yqVeM43VYAqF5tj Rwx05OFYzWWxQ42RPNP3PtteXOdSWYNTDUIpfW1D1Q== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelledgudefkecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepofgfggfkjghffffhvfevufgtse httdertderredtnecuhfhrohhmpedftfhitghkucfnuhhpthhonhdfuceomhgrihhlsehr ihgtkhhluhhpthhonhdrnhgrmhgvqeenucggtffrrghtthgvrhhnpeffgefgudeuvdejhe etffeghffgvefghedugeevheekgeegvdeuheejuddvuedvtdenucffohhmrghinheptgho nhhtvgigthdrohhrghdprhgvlhhirggslhgvrdhorhhgpdhorhhgmhhouggvrdhorhhgpd hlihgsvghrrghprgihrdgtohhmnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghm pehmrghilhhfrhhomhepmhgrihhlsehrihgtkhhluhhpthhonhdrnhgrmhgv X-ME-Proxy: Feedback-ID: i0143436f:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id E21D31700097; Thu, 14 Dec 2023 15:42:24 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-1283-g327e3ec917-fm-20231207.002-g327e3ec9 MIME-Version: 1.0 Message-Id: <2cdfefbf-e9e3-4aeb-a410-1ff3a9d6168e@app.fastmail.com> In-Reply-To: <87jzpmqiy0.fsf@localhost> References: <118435e8-0b20-46fd-af6a-88de8e19fac6@app.fastmail.com> <87edkwsafe.fsf@localhost> <87cywh2ad6.fsf@localhost> <87jzpmqiy0.fsf@localhost> Date: Thu, 14 Dec 2023 20:42:02 +0000 From: "Rick Lupton" To: "Ihor Radchenko" Cc: "Y. E." Subject: Re: [PATCH] org-id: allow using parent's existing id in links to headlines Content-Type: text/plain Received-SPF: pass client-ip=66.111.4.27; envelope-from=mail@ricklupton.name; helo=out3-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 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_LOW=-0.7, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, 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-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -9.79 X-Spam-Score: -9.79 X-Migadu-Queue-Id: 3442332752 X-Migadu-Scanner: mx10.migadu.com X-TUID: Q07zZleIoKFZ Dear Ihor, Thanks for taking a look at the patch and for your feedback. Re various docstrings, documenting new optional argument -- I will try to address these. Other comments/questions below. On Sun, 10 Dec 2023, at 1:35 PM, Ihor Radchenko wrote: > "This change..." part sounds like a commit message, not a NEWS item. I think there are lots of other examples that are written like this in ORG-NEWS (but I agree my sentence was unnecessary and have removed it). >> +(defcustom org-id-link-use-context t >> + "Non-nil means org-id links from `org-id-store-link' contain context. >> +\\ >> +A search string is added to the id with \"::\" as separator and >> +used to find the context when the link is activated by the >> +command `org-open-at-point'. When this option is t, the entire >> +active region is be placed in the search string of the file link. >> +If set to a positive integer N, only the first N lines of context >> +are stored. > > It does not look like integer value is respected in the patch. You're right. Do you have a preference between (a) sticking to this docstring, which creates the possibility of using different numbers of lines for id: and file: links' context, and makes the code slightly more complicated, but keeps the meaning of `org-link-context-for-files' specifically `for files'; or (b) Always use `org-link-context-for-files' to set the number of lines of context used for all links; `org-id-link-use-context' is just a boolean. The code is simpler. ? >> - (let ((id (org-entry-get epom "ID"))) >> + (let ((id (org-entry-get epom "ID" inherit))) > > This makes your description of INHERIT argument slightly inaccurate - for > `org-entry-get', INHERIT can also be a special symbol 'selective. Good point; I think the answer is to force INHERIT to t or nil, rather than documenting and continuing to accept 'selective (when INHERIT is used, it should definitely take effect). >> -(defun org-id-store-link () >> +(defun org-id-store-link (interactive?) > > Please make this new argument optional and document the argument in the > docstring and NEWS. Non-optional new argument is a breaking change that > may break third-party code. Oops, yes -- but in fact this argument is only needed on `org-id-store-link-maybe' (as below), so I can remove it again here. >> "Store a link to the current entry, using its ID. >> >> +See also `org-id-link-to-org-use-id`, >> +`org-id-link-use-context`, >> +`org-id-link-consider-parent-id`. >> + >> If before first heading store first title-keyword as description >> or filename if no title." >> - (interactive) >> - (when (and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode)) >> - (let* ((link (concat "id:" (org-id-get-create))) >> + (interactive "p") >> + (when (and (buffer-file-name (buffer-base-buffer)) >> + (derived-mode-p 'org-mode) >> + (or (eq org-id-link-to-org-use-id t) > > I do not like this change - `org-id-store-link' is not only used by > `org-store-link'. Suddenly honoring `org-id-link-to-org-use-id' will be > a breaking change. Instead, I suggest you to write a wrapper function, > like `org-id-store-link-maybe' and use it as :store id link property. > That function will call `org-id-store-link' as necessary according to > user customization. Ok, yes that makes sense. >> + ;; Prefix to `org-store-link` negates preference from `org-id-link-use-context`. >> + (when (org-xor current-prefix-arg org-id-link-use-context) > > This is not reliable. `org-id-store-link' may be called from a completely > different command, not `org-store-link'. Then, the effect of prefix > argument will be unexpected. You should instead process prefix argument > right in `org-store-link' by let-binding `org-id-link-use-context' > around the call to `org-id-store-link'. Now that `org-id-store-link' is called via :store link property, `org-store-link` does not have special logic for org-id, which I thought was an improvement, so it would be a step backwards to add in special logic for `org-id-link-use-context'? Instead, I think this logic could be in `org-id-store-link-maybe' as above. That is, it is safe to take account of `current-prefix-arg' within a link :store function, and assume it represents prefix args as used with `org-store-link'? > >> + (pcase (org-link-precise-link-target id-location) > > Why not passing the RELATIVE-TO argument? The `id-location' is the RELATIVE-TO argument. Or do I misunderstand you? >> (defun org-id-open (id _) >> "Go to the entry with id ID." >> - (org-mark-ring-push) >> - (let ((m (org-id-find id 'marker)) >> - cmd) >> + (let* ((option (and (string-match "::\\(.*\\)\\'" id) >> + (match-string 1 id))) >> + (id (if (not option) id >> + (substring id 0 (match-beginning 0)))) >> + m cmd) >> + (org-mark-ring-push) >> + (setq m (org-id-find id 'marker)) > > This means that the existing IDs that happen to contain :: will be > broken. For such IDs, we should (1) document the problem in the news; > (2) try harder to match them calling `org-id-find' with all the possible > ID values until one matches. Good point, ok, I'll try this. >> + (when option >> + (org-link-search option)) >> (org-fold-show-context))) > > `org-link-search' does not always search from point. So, you may end up > matching, for example, a duplicate CUSTOM_ID above. > Moreover, regular expression match option will be broken - > `org-link-search' creates sparse tree in the whole buffer and will > disregard the ID part of the link. I suspect that you will need to make > dedicated modifications to `org-link-search' as well in order to > implement opening ID links with search option cleanly. Thanks, yes. It looks to me (from the code and some testing) that narrowing to the target heading first before calling `org-link-search' does the right thing. Was there a particular reason you thought `org-link-search' would need to be changed? Thanks, Rick > > -- > Ihor Radchenko // yantar92, > Org mode contributor, > Learn more about Org mode at . > Support Org development at , > or support my work at