Re: [PATCH] Allow external libraries (org-roam) to supply org-id locations

2024-05-02 Thread Rick Lupton
I have been using the simpler version using advice (without my patch).

On reflection this seems ok to me -- I don't think there's really any need for 
my proposed patch, unless it turns out in future that there are multiple 
packages like org-roam that need to supply org-id-finding abilities in a 
well-defined order.  So I suggest cancelling this.

I just proposed this as a change to org-roam:

https://github.com/org-roam/org-roam/pull/2432

Best
Rick

On Sat, 20 Apr 2024, at 12:07 PM, Ihor Radchenko wrote:
> Ihor Radchenko  writes:
>
>> Also, before we merge your patch, may I know if you discussed this
>> change with org-roam developers? If they do not want to use the proposed
>> hook, there is no reason to add it to Org mode.
>
> It has been over a month since the last message in this thread.
> May I know if there is any progress?
>
> -- 
> Ihor Radchenko // yantar92,
> Org mode contributor,
> Learn more about Org mode at .
> Support Org development at ,
> or support my work at 



Attributes on images (was:: Experimental public branch for inline special blocks)

2024-04-14 Thread Rick Lupton
Thanks for taking the time to comment thoroughly on this which seems generally 
like it would be a good improvement.

On Tue, 9 Apr 2024, at 9:52 AM, Ihor Radchenko wrote:
> 2. Allow attaching auxiliary attributes to the on object level, as an
>equivalent of affiliated keywords on element level
>
>- For example, we should allow assigning height per-link without the
>  awkward kludge we have with special handling of
> 
>  #+attr_html: :height 300
>  [[file:image.png]] has a height of 300, but what if we want a
>  different height in [[file:another-image.png]]?
>
>  We can thus do
> 
>  #+attr_html: :height 300
>  [[file:image.png]] has a height of 300, but what if we want a
>  different height in @@[:html-height 300]{[[file:another-image.png]]}?
>
>  Note how @@{...} markup assigns attributes to objects inside - the
>  attributes should be somehow inherited.

This way of assigning a height to the image seems odd to me.  Mostly, the 
attributes specified by the inline block apply to the block, not the contents, 
so wouldn't this case be potentially surprising?

Both of these examples mean different things in HTML, and it seems like you 
might want to create either -- how would you control which was produced using 
the "@@[:html-height 300]{[[file:another-image.png]]}" syntax?





Instead, I wonder if the problem is that the way of inserting an image using a 
link itself.  If you need more control, could there be a special "img" inline 
special block which can handle additional attributes?

For example: 

@img[:height 300]{image.png} has a height of 300, and we can also have images 
with different heights and attributes like @img[:height 400 :alt "An 
image"]{another-image.png}.

Or, if using the original syntax, perhaps the attribute should be explicitly 
:img-attr or :img-height to resolve the ambiguity about which element is being 
targetted?

Rick



Re: [PATCH] Allow external libraries (org-roam) to supply org-id locations

2024-03-17 Thread Rick Lupton
Thanks, I hadn't understood the significance of named functions vs variables 
for advice.  But now I realise this is not solving the same problem as the 
original patch.

The point is that I was thinking org-roam should advise/modify/hook the 
specific function `org-id-find' [to find ids in its database] but NOT the more 
general `(org-link-get-parameter "id" :follow)' [because `org-id-open' has a 
lot of extra logic for search-strings, backwards compatibility, where to open 
the new location, which it would not be reasonable to duplicate].

Is there a way to do this using the approach you are suggesting?



Re: [PATCH] Allow external libraries (org-roam) to supply org-id locations

2024-03-16 Thread Rick Lupton
On Wed, 13 Mar 2024, at 12:30 PM, Ihor Radchenko wrote:
> I think that we can do it simpler. [...]
> The idea is to use Emacs' advice machinery to allow third-party code
> alter the functions stored in link parameters.

I was avoiding this because I thought it was only recommended (in the elisp 
manual) to use advice directly by users, not in libraries (like, I assume, 
org-roam):

> If you are writing code for release, for others to use, try to avoid 
> including advice in it. If the function you want to advise has no hook to do 
> the job, please talk with the Emacs developers about adding a suitable hook. 
> Especially, Emacs’s own source files should not put advice on functions in 
> Emacs. (There are currently a few exceptions to this convention, but we aim 
> to correct them.) It is generally cleaner to create a new hook in foo, and 
> make bar use the hook, than to have bar put advice in foo.

(https://www.gnu.org/software/emacs/manual/html_node/elisp/Advising-Named-Functions.html)

But I don't mind either way.  I agree your approach is simpler if it's a 
reasonable way for a third party library like org-roam to extend the org id 
functions.

Thanks,
Rick



[PATCH] Allow external libraries (org-roam) to supply org-id locations

2024-03-12 Thread Rick Lupton
Hi all,

Since updating the org-id code [1] to use the standard org-link registered 
functions instead of hard-coding org-id opening, I have a problem using 
org-roam.

org-roam overwrites the :follow function for "id" links from the build-in 
`org-id-open' to its own `org-roam-id-open' 
(https://github.com/org-roam/org-roam/blob/8667e441876cd2583fbf7282a65796ea149f0e5f/org-roam-id.el#L91).
  The only change between these functions is to insert a call to try 
`org-roam-id-find' before trying `org-id-find', which uses org-roam's cached 
sqlite database to find the id 
(https://github.com/org-roam/org-roam/blob/8667e441876cd2583fbf7282a65796ea149f0e5f/org-roam-id.el#L70-L71)

As well as being messy, my specific problem is that the improvements to open 
search strings in org-id links are no longer enabled when org-roam is loaded.

It seems reasonable that a library might want to provide its own way of 
locating org-ids.  The attached patch adds a new hook variable 
`org-id-find-functions` which contains functions doing the same job as 
`org-id-find' that should be tried first.  Then all org-roam needs to do is add 
its `org-roam-id-find' to the hook.

Does this seem like a good idea?

Thanks,
Rick

[1] Link: 
https://list.orgmode.org/118435e8-0b20-46fd-af6a-88de8e19f...@app.fastmail.com/


0001-lisp-org-id.el-add-hook-org-id-find-functions.patch
Description: Binary data


Re: org-id-locations as a large-scale database store?

2024-03-12 Thread Rick Lupton
You might be interested in https://www.orgroam.com/ which is a broadly similar 
idea to org-brain but uses a SQLite database to track id locations and links.



Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-02-24 Thread Rick Lupton
Thanks for your help with it!

On Sat, 24 Feb 2024, at 1:02 PM, Ihor Radchenko wrote:
> Bastien Guerry  writes:
>
>>> "Rick Lupton"  writes:
>>>
>>>> On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
>>>>> May you please update on your FSF copyright assignment status?
>>>>
>>>> I believe the agreement is all signed and completed. 
>>>
>>> Bastien, may your please check FSF records?
>>
>> Done, and it is well in order.
>
> Thanks for checking!
> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6e7e0b2cd
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=95554543b
>
> and added a record to the contributor list.
> https://git.sr.ht/~bzg/worg/commit/0ccaf58a
>
> Rick, thanks for your contribution!
>
> -- 
> Ihor Radchenko // yantar92,
> Org mode contributor,
> Learn more about Org mode at <https://orgmode.org/>.
> Support Org development at <https://liberapay.com/org-mode>,
> or support my work at <https://liberapay.com/yantar92>



Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-02-09 Thread Rick Lupton
On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
> May you please update on your FSF copyright assignment status?

I believe the agreement is all signed and completed. 

Thanks
Rick



Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-02-08 Thread Rick Lupton
On Thu, 8 Feb 2024, at 1:02 PM, Ihor Radchenko wrote:
> I have some thoughts about rewording your changes to the manual and
> ORG-NEWS. See the attached patch on top of yours.

Thanks, makes sense -- wasn't sure whether to keep this as a separate patch or 
not, I have squashed into the attached updated version.

> [minor points on commit messages]

Fixed these.

> The new optional argument to a public function should be announced in 
> ORG-NEWS.

Added.

>> + (new-heading-level (if new-heading-container
>> +(+ 1 (org-element-property :level 
>> new-heading-container))
>
> What if new-heading-container is not a heading?
>
>> +  1)))
>> +(goto-char new-heading-position)
>
> This is err when container ends after narrowed region boundary.

Added checks for these.

>> +(defun org-link-precise-link-target ()
>> ...
>> +  (cond
>> +   (name
>> +(list name
>> +  name
>> +  (org-element-begin element)))
>
> It would make sense to use #+caption as default description when available.

Maybe... But I had a little look and it seems complicated, since caption is a 
parsed property, it's not clear to me how to get a plain string in a simple 
way. And there could be a long and a short caption, over multiple lines. If the 
caption is long, it wouldn't make a good link description anyway.

The current behaviour is the same as it was before, so maybe we can leave this 
as a future enhancement if wanted?


0001-lisp-org.el-org-insert-heading-Allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Add-search-strings-inherit-parent-IDs.patch
Description: Binary data


Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-02-08 Thread Rick Lupton
On Sat, 3 Feb 2024, at 1:10 PM, Ihor Radchenko wrote:
> I'd prefer to avoid using global variables here.
> `org-entry-property-inherited-from' dates to pre-lexical binding times
> and is a potential source of subtle bugs if several `org-entry-get'
> calls happen unexpectedly to the code, changing
> `org-entry-property-inherited-from' multiple times.
>
> Instead, I suggest changing the return value of
> `org-link-precise-link-target' to a list that includes marker in
> addition to search string and description.

Makes sense -- I changed it to work that way and it is neater.

I returned simply the buffer position rather than a marker, since it is always 
in the current buffer, and avoids needing to worry about cleaning up the marker 
when finished or if not of interest.

> It looks like we cannot simply rely on narrowing to determine the
> created heading level.

I think you're right.  I have extended `org-link-search' to accept an optional 
argument describing the org element where newly created headings should go as 
subheadings.

My thought was that this was not significantly more complicated than just 
passing the numeric level for new headings, but actually more flexible (e.g. 
you could if you wanted (with additional future elisp) create missing headings 
as part of a "To be filed" subtree within the file, rather than always at the 
end).

Does that look ok?

[is it useful to keep attaching the unchanged first patch so they are available 
as a set?]

Thanks
Rick

0001-lisp-org.el-org-insert-heading-allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Add-search-strings-inherit-parent-IDs.patch
Description: Binary data


[PATCH] lisp/ol.el: Improve docstring

2024-02-08 Thread Rick Lupton
On Sat, 3 Feb 2024, at 1:10 PM, Ihor Radchenko wrote:
> In my testing, I used #+name: as link target.
> However, what I missed is that #+name targets are matched even when
> `org-link-search-must-match-exact-headline' is set to 'query-to-create.
> The docstring is not accurate there and must be updated.

How about this?

0001-lisp-ol.el-Improve-docstring.patch
Description: Binary data


Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-02-01 Thread Rick Lupton
On Thu, 1 Feb 2024, at 12:13 PM, Ihor Radchenko wrote:
> The patch does not apply onto the latest main. May you please update it?

I have rebased onto the latest main. It changes quickly!

(there were no conflicts during the rebase, which I'd have thought would mean 
the patches shouldn't be a problem to apply? It the problem was something else, 
please let me know)

0001-lisp-org.el-org-insert-heading-allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Extend-links-with-search-strings-inherit-p.patch
Description: Binary data


Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-01-31 Thread Rick Lupton
On Mon, 29 Jan 2024, at 1:00 PM, Ihor Radchenko wrote:
>>> 3. Consider
>>>(setq org-id-link-consider-parent-id t)
>>>(setq org-id-link-to-org-use-id t)
>>>
>>>Then, create a new empty Org file
>>>M-x org-store-link with create a top-level properties drawer with ID
>>>and store the link. However, that link will not be a simple ID link,
>>>but also have ::PROPERTIES search string, which is not expected.
>>
>> This is because it is trying to link to the current line of the file, which 
>> contains the text "PROPERTIES".  On main, with (setq 
>> org-id-link-to-org-use-id nil), you see the equivalent behaviour (a link to 
>> [[file:test.org:::PROPERTIES:]]) when point is before the first heading.  
>> So, this seems consistent with non-org-id links?
>
> No. Do note that my instructions start from _empty_ file. With
> org-id-link-to-org-use-id, PROPERTIES drawer is not created. This is
> different from what happens with your patch - it is unexpected in your
> patch that the search string is added for text that did not exist in the
> buffer previously.

I see.  Updated to get the search string first, before the possible properties 
draw appears.

To make this work I changed `org-link-precise-link-target': instead of 
accepting the RELATIVE-TO argument and rejecting unsuitable targets internally, 
it now sets a marker `org-link-precise-target-marker' showing where the target 
that was found is, so the caller can decide if the found target is suitable.  I 
copied the approach from `org-entry-property-inherited-from', hope that doesn't 
cause any other issues.

> That's a good catch.
> The fact that links stored via `org-store-link' cannot be open with
> default settings is not good.
> Also, your patch disregards this setting - it should not match
> non-headline search strings with the default value of
> `org-link-search-must-match-exact-headline'.

`org-link-search-must-match-exact-headline' affects `org-link-search', which is 
called by `org-id-open' -- so I think the behaviour for these org-id links 
should be the same as for other file links? Am I missing something?

Or, maybe you mean links that rely on 
`org-link-search-must-match-exact-headline' should not be stored.  That would 
seem reasonable, but also doesn't need to be part of these changes here?

> Probably, changing the default value of
> `org-link-search-must-match-exact-headline' to nil is due.

It seems like the behaviour below would be desirable, but doesn't currently 
exist with any setting of `org-link-search-must-match-exact-headline'?

(org-link-search "plain text")  -->  fuzzy search for all text
(org-link-search "*heading")-->  search only headings, optionally creating 
if missing

>> Subject: [PATCH 2/2] org-id.el: Extend links with search strings, inherit
>>  parent IDs
>
> I ran make test, and it looks like one test is failing with your patch:

Oops, fixed now I think.

> `org-context-in-file-links' is an obsolete name. Use
> `org-link-context-for-files'.
>
> Also, please add `org-id-link-use-context' to #+vindex.

Updated

> Please update the docstring of `org-store-link-functions' to specify
> that an argument is passed to :store functions.

Updated

>> -  (org-insert-heading nil t t)
>> +  ;; Find appropriate level for new heading
>> +  (let ((level (save-excursion
>> + (goto-char (point-min))
>> + (+ 1 (or (org-current-level) 0)
>
> This is fragile. You assume that `point-min' always contains a heading.
> That may or may not be the case - `org-link-search' may be called by
> third-party code that does not care about setting narrowing in certain
> ways.

I don't think it's a problem. (org-current-level) returns something suitable 
whether or not point-min contains a heading. Both the situations below seem 
reasonable choices for the level of the newly created heading at the end:

---start of narrowing---
Text
* H1
** H2
* A new level 1 heading is created at the end
---end of narrowing---

---start of narrowing---
* H1
** H2
** A new level 2 heading is created at the end
---end of narrowing---

(this is how it currently works, unless I'm missing something)

>> +(defun org-link-precise-link-target ( relative-to)
>> +  "Determine search string and description for storing a link.
>> +
>> +If a search string (see 'org-link-search') is found, return cons
>
> Quoting: `org-link-search'.

Fixed

>> +   (let* ((element (org-element-at-point))
>> +  (name (org-element-property :name element))
>> +  (heading (org-element-lineage element 'headline t))
>
> What about inlinetasks?

I added inlinetasks to the element types, so they are picked up the same as 
headlines now.

>> +  (custom-id (org-entry-get nil "CUSTOM_ID")))
>
> May as well pass HEADING as the first argument of `org-entry-get'. It
> will be slightly more efficient.

Ok

>> +(org-link--add-to-stored-links link desc)
>> +;; In org 

Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-01-28 Thread Rick Lupton
Hi,

Thanks for trying it out.  Updated patches attached, comments below.

On Mon, 18 Dec 2023, at 12:27 PM, Ihor Radchenko wrote:
> I played around with the patch a bit and found a couple of rough edges:
>
> 1. When I try to open a link to non-existing search target, like
>, I get a query to create a new
>heading. If I reply "yes", a new heading is created. However, the
>heading is created at the end of the file and is always level 1,
>regardless of the "some-id" parent context.
>It would make more sense to create a new heading at the end of the
>id:some-id subtree.

Fixed in updated patches -- first patch adds generic new flexibility to 
`org-insert-heading', second patch uses it so new headings now added at correct 
level at the end of the id:sub-id subtree.

> 2. Consider the following setting:
>(setq org-id-link-consider-parent-id t)
>(setq org-id-link-to-org-use-id 'create-if-interactive-and-no-custom-id)
>
>Then, create the following Org file
>
> * Sub
> * Parent here
> ** This is test
> :PROPERTIES:
> :ID:   fe40252e-0527-44c1-a990-12498991f167
> :END:
>
> *** Sub 
> :PROPERTIES:
> :CUSTOM_ID:   subid
> :END:
>
>When you M-x org-store-link, the stored link has ::*Sub instead of
>the expected ::#subid

Updated so that search strings prefer custom-ids (::#subid) to headline matches 
(::*Sub).  This makes this example behave as you expect.

The correct behaviour of org-store-link doesn't seem totally obvious to me 
about id vs custom-id links.  Currently org-store-link has special logic to 
store TWO links (one , one ) when a CUSTOM_ID 
is present. In the manual, it says:

 If the headline has a ‘CUSTOM_ID’ property, store a link to this
 custom ID.  In addition or alternatively, depending on the value of
 ‘org-id-link-to-org-use-id’, create and/or use a globally unique
 ‘ID’ property for the link(1).  So using this command in Org
 buffers potentially creates two links: a human-readable link from
 the custom ID, and one that is globally unique and works even if
 the entry is moved from file to file.  The ‘ID’ property can be
 either a UUID (default) or a timestamp, depending on
 ‘org-id-method’.  Later, when inserting the link, you need to
 decide which one to use.

That refers to ID links specifically, but now, using the generic link store 
functions, there is only the possibility to store one link type, so it's not 
possible to neatly keep exactly the same behaviour (i.e. for ID links but not 
for other external link types).

I think the intention of what's described in the manual is to distinguish 
"human-readable" vs "persistent id" links.  There could be other types of 
"persistent id" links apart from org-id links, such as mu4e: links to email 
message-ids.  Therefore I've updated org-store-link to simply store a 
 link as an additional option, whether or not the 
first matched link was an org-id link (this is the current behaviour) or 
another external link type (this is changed behaviour).

Added a note to ORG-NEWS about this.

> 3. Consider
>(setq org-id-link-consider-parent-id t)
>(setq org-id-link-to-org-use-id t)
>
>Then, create a new empty Org file
>M-x org-store-link with create a top-level properties drawer with ID
>and store the link. However, that link will not be a simple ID link,
>but also have ::PROPERTIES search string, which is not expected.

This is because it is trying to link to the current line of the file, which 
contains the text "PROPERTIES".  On main, with (setq org-id-link-to-org-use-id 
nil), you see the equivalent behaviour (a link to 
[[file:test.org:::PROPERTIES:]]) when point is before the first heading.  So, 
this seems consistent with non-org-id links?

(these links don't actually work with the default value of 
`org-link-search-must-match-exact-headline', but I think that's a separate 
issue).

>> +  #+vindex: org-id-link-consider-parent-id
>> +  When ~org-id-link-consider-parent-id~ is ~t~, parent =ID= properties
>> +  are considered.  This allows linking to specific targets, named
>> +  blocks, or headlines (which may not have a globally unique =ID=
>> +  themselves) within the context of a parent headline or file which
>> +  does.
>
> It would be nice to add an example, similar to what you did in the docstring.

Added.

>
>> -(defun org-man-store-link ()
>> +(defun org-man-store-link ( _interactive?)
>>"Store a link to a man page."
>>(when (memq major-mode '(Man-mode woman-mode))
>>  ;; This is a man page, we do make this link.
>> @@ -21312,13 +21324,15 @@ A review of =ol-man.el=:
>
> Please, update the actual built-in :store functions in lisp/ol-*.el to
> handle the new optional argument as well.

Updated.

>> + =org-link= store functions are passed an ~interactive?~ argument
>> +
>> +The ~:store:~ functions set for link types using
>> +~org-link-set-parameters~ are now passed an ~interactive?~ argument,
>> +indicating whether 

Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

2024-01-02 Thread Rick Lupton
On Mon, 18 Dec 2023, at 12:27 PM, Ihor Radchenko wrote:
> I played around with the patch a bit and found a couple of rough edges:
>
> 1. When I try to open a link to non-existing search target, like
>, I get a query to create a new
>heading. If I reply "yes", a new heading is created. However, the
>heading is created at the end of the file and is always level 1,
>regardless of the "some-id" parent context.
>It would make more sense to create a new heading at the end of the
>id:some-id subtree.

Thanks for the comments. On this point, I'd like to modify `org-insert-heading' 
to allow for choosing the level of the newly inserted heading, but first wanted 
to check if you have a preference for how to change it.


I think it would be simplest to change the current:

(defun org-insert-heading ( arg invisible-ok top)
  "...When optional argument TOP is non-nil, insert a level 1 heading, 
unconditionally."

to:

(defun org-insert-heading ( arg invisible-ok level)
  "...When optional argument LEVEL is a number, insert a heading at that level. 
 For backwards compatibility, when LEVEL is non-nil but not a number, insert a 
level-1 heading."

but that is not totally backwards compatible -- is that ok?


If it should be completely backwards compatible, alternatively could add an 
additional optional argument:

(defun org-insert-heading ( arg invisible-ok top top-level)
  "...When optional argument TOP is non-nil, insert a top-level heading, 
unconditionally.  When TOP-LEVEL is non-nil, use that level, otherwise level 1."


Alternatively I could preserve the intention of TOP but add a special value to 
change what "top-level" means, so the docstring would become something like 
this:

"When optional argument TOP is non-nil, insert a top-level
heading, unconditionally.  Specifically, when TOP is `relative',
\"top-level\" means one level deeper than the outline level at
minimum point position (respecting any narrowing of the buffer).
Otherwise, \"top-level\" means level 1."

(the motivation for this is that when the buffer is narrowed to the subtree 
with the matching ID, the new heading will be created at the appropriate level).


Best
Rick



[PATCH v2] org-id: allow using parent's existing id in links to headlines

2023-12-17 Thread Rick Lupton
Please find attached updated patch which I think addresses all the points 
discussed.  Let me know if you see any further changes needed.

Thanks,
Rick


0001-org-id.el-Extend-links-with-search-strings-inherit-v2.patch
Description: Binary data


Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-12-15 Thread Rick Lupton
On Fri, 15 Dec 2023, at 12:55 PM, Ihor Radchenko wrote:
> No, it is generally not safe. For a different reason.
>
> Let me illustrate with an example:
>
> ...
>
> Conclusion: It is unsafe to use `current-prefix-arg' value. We need to
> pass this information some other way.
>
> The way I proposed is actually not any special for ID links. What I
> meant it to let-bind `org-link-context-for-files' around the whole call
> to `org-store-link-functions', so that the custom :store functions will
> get access to the adjusted value of `org-link-context-for-files'.
> Does this explanation make more sense?

Thanks for the example and explanation. Yes that does make sense, mostly. I 
assume this would look like this in org-store-link:

(let ((org-link-context-for-files (org-xor org-link-context-for-files (equal 
arg '(4
(...call store link functions...))

The meaning of `org-link-context-for-files' is then shifting from being "should 
file: links include search strings (and how much should be included when the 
region is active)" from "should any link that supports search strings include 
them (and how much should be included when the region is active)". Is it 
necessary to rename it to reflect this? (e.g. to `org-link-use-context' or 
similar).

It's also then less clear what the role of `org-id-link-use-context' is and how 
it interacts with `org-link-context-for-files'. I had included 
`org-id-link-use-context' to give a way to opt out of the new behaviour (i.e. 
using the update discussed above, a search string is added if (and 
org-link-context-for-files org-id-link-use-context) ). But perhaps this is also 
unnecessarily complicated, and `org-id-link-use-context' could be removed again 
completely?

> I will update the docstring of
> `org-link-search' to explicitly specify that it is searching within the
> accessible portion of the buffer and update the callers to account for
> this.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=89164e605
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5c543cd9d
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=cb71bde7c
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=63ef7b924
>
> But does your code do narrowing? I did not notice it.

Not in the patch I sent, I added it later after you pointed this out.

I'll send an updated patch next.

Thanks,
Rick



Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-12-14 Thread Rick Lupton
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 

Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-12-04 Thread Rick Lupton
Hi,

I can’t see this patch listed at https://tracker.orgmode.org/ so just wanted to 
check it hasn’t got lost?

Thanks
Rick

On Sun, 19 Nov 2023, at 3:21 PM, Rick Lupton wrote:
> Here's an updated patch, which adds (optional) search strings to ID 
> links, and the option to inherit ID targets from parent headline / the 
> top level file properties.  I've also updated ORG-NEWS and the manual, 
> and added tests.
>
> I think I've fixed all the issues with my first patch about which 
> headline gets used for the description when inheriting IDs, what 
> happens if there is no ID, etc.
>
>> Ideally, we should have all the necessary logic to store the link within 
>> `org-id-store-link' and then use `org-link-set-parameters' to configure id 
>> links.
>> ...
>> I think that we need to make a change in the rules for :store functions. 
>> `interactive?' may be passed as the argument to these functions.
>
> I've also moved the org-id specific logic from `org-store-link` to 
> `org-id-store-link`, and added the `interactive?` argument to link 
> store functions as discussed.
>
>>> So my question is: should search strings be added to all org-id links?
>> Sounds as a reasonable default, but users should have an option to revert to 
>> previous behaviour with heading id being stored.
>
> The default value for the new option `org-id-link-use-context` is `t`, 
> but it can be set to `nil` (or disabled with a prefix argument to 
> `org-store-link` temporarily).  This is a change in default behaviour 
> when storing ID links with point at a subheading, named block, or 
> target, or with an active region.
>
> The option `org-id-link-consider-parent-id` I've left with a default 
> value of `nil`, since I'm not sure if everyone will want this behaviour.
>
> Thanks
> Rick
>
> Attachments:
> * 0001-org-id.el-Extend-links-with-search-strings-inherit-p.patch



Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-11-19 Thread Rick Lupton
Here's an updated patch, which adds (optional) search strings to ID links, and 
the option to inherit ID targets from parent headline / the top level file 
properties.  I've also updated ORG-NEWS and the manual, and added tests.

I think I've fixed all the issues with my first patch about which headline gets 
used for the description when inheriting IDs, what happens if there is no ID, 
etc.

> Ideally, we should have all the necessary logic to store the link within 
> `org-id-store-link' and then use `org-link-set-parameters' to configure id 
> links.
> ...
> I think that we need to make a change in the rules for :store functions. 
> `interactive?' may be passed as the argument to these functions.

I've also moved the org-id specific logic from `org-store-link` to 
`org-id-store-link`, and added the `interactive?` argument to link store 
functions as discussed.

>> So my question is: should search strings be added to all org-id links?
> Sounds as a reasonable default, but users should have an option to revert to 
> previous behaviour with heading id being stored.

The default value for the new option `org-id-link-use-context` is `t`, but it 
can be set to `nil` (or disabled with a prefix argument to `org-store-link` 
temporarily).  This is a change in default behaviour when storing ID links with 
point at a subheading, named block, or target, or with an active region.

The option `org-id-link-consider-parent-id` I've left with a default value of 
`nil`, since I'm not sure if everyone will want this behaviour.

Thanks
Rick


0001-org-id.el-Extend-links-with-search-strings-inherit-p.patch
Description: Binary data


Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-11-09 Thread Rick Lupton
On Tue, 25 Jul 2023, at 8:43 AM, Ihor Radchenko wrote:
> Ideally, we should have all the necessary logic to store the link within
> `org-id-store-link' and then use `org-link-set-parameters' to configure
> id links.

I agree this would be neater, but looking at how this would work, I have a 
question:

Behaviour in `org-store-link` currently depends on the `interactive?` argument, 
e.g. in this logic

(and interactive?
 (or (eq org-id-link-to-org-use-id 'create-if-interactive)
 (and (eq org-id-link-to-org-use-id
  'create-if-interactive-and-no-custom-id)
  (not custom-id

To move this logic to `org-id-store-link`, is there a way that 
`org-id-store-link` can tell whether `org-store-link` was called (a) 
interactively, or (b) with the `interactive?` argument true?

Thanks
Rick



Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-11-04 Thread Rick Lupton
I realised there is another question here about how search strings are used in 
org-id links.

Consider this example file:


* Heading
:PROPERTIES:
:ID:   06E767E6-6145-45EB-B736-D350449126EC
:END:

#+name: named-thing
#+begin_example
Hi!
#+end_example


By default (`org-id-link-to-org-use-id` is nil), with point on `#+name: 
named-thing`, calling `org-store-link` will give a link like 
`[[file:test.org::named-thing][named-thing]]` which leads directly to the named 
example block. Different uses can also lead to search strings which link to 
headings, selected text in the region, or the current line's text.

When `org-id-link-to-org-use-id` is non-nil, none of this happens -- calling 
`org-store-link` anywhere within the subtree will result in a link 
`[[id:06E767E6-6145-45EB-B736-D350449126EC][Heading]]` with no additional 
search string.

My previous patch changes the behaviour when `org-id-link-to-org-use-id` has a 
new value (`inherit`) in two ways:

(a) org-ids from parent headings are considered when choosing the ID to link 
to, and 
(b) search strings are added to the link

But these are actually two independent things. So my question is: should search 
strings be added to all org-id links?

This would make org-id links more powerful/precise (because you can link to 
more precise locations within the subtree), and simplifies the code in 
`org-store-link` in my patch (because point [b] above would apply to all org-id 
links, not just the new 'inherit ones), but it could change the behaviour when 
calling `org-store-link` with an active region or when point is on a named 
element.

Depending on the answer, I can update the patch accordingly.

Thanks,
Rick



Re: IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines)

2023-07-28 Thread Rick Lupton
I can see this being useful in general, but not avoiding the need for my patch. 
Org links using search strings already strike a good compromise between working 
with arbitrary plain text, and allowing links to specific locations. When a 
search string is enough to find the thing you want to link to, there’s no need 
to add more IDs manually. 

If this is already intended to be an unrelated discussion then feel free to 
ignore this comment!

On Thu, 27 Jul 2023, at 8:42 AM, Ihor Radchenko wrote:
> Samuel Wales  writes:
>
>> ...  but what if those smaller things
>> could have ids without drawers?  id markers.  then changes in
>> surrounding text would not break anything.
>
> I recall similar idea raised in
> https://list.orgmode.org/orgmode/cajniy+ovd0ncwzztpit5t7wvsblbgllxzmpub5tgq3gshsg...@mail.gmail.com/
>
> But there was not much interest.
>
> It was pointed that we already have link targets, although they are not
> global. Making link targets global is doable.
>
> -- 
> Ihor Radchenko // yantar92,
> Org mode contributor,
> Learn more about Org mode at .
> Support Org development at ,
> or support my work at 



Re: [PATCH] org-id: allow using parent's existing id in links to headlines

2023-07-28 Thread Rick Lupton
Hi Ihor,

Thanks for the comments, I will take a look. A question below. 

On Wed, 26 Jul 2023, at 9:10 AM, Ihor Radchenko wrote:
>
> I am looking at it from an opposite direction: we already have file:
> links with ::search term, but file is not a very reliable link anchor.
> File ID will persist even when the file is moved. So, instead of having
> something like , we should better also
> provide  with ID defined in the top-level property
> drawer. ID being some sub-heading is then a natural extension of the
> same idea.

This is a good description of the motivation from my point of view. 

> What about inherited CUSTOM_ID?

I’m not sure what you mean. 

Are you thinking of CUSTOM_ID links, and whether they would behave consistently 
with a search string to this proposal? Like: [[custom-id:my-id::*H2][H2]]

Or using custom id as a search string? Like: [[id:abc::#my-id][Description]]

Thanks
Rick



[PATCH] org-id: allow using parent's existing id in links to headlines

2023-07-24 Thread Rick Lupton
Hi,

Here is a small new feature for org-id that I have been using and finding 
useful. The patch adds the option to look for ancestors of the current headline 
that have an ID defined and use that together with a link search string to link 
to specific headlines, without needing every single headline to have its own ID.

For example if you have:

#+begin_example
* H1
:PROPERTIES:
:ID: abc
:END:

** H2
Link to here
#+end_example

with `org-id-link-to-org-use-id' set to `t`, the result of org-store-link will 
be that "H2" has a new id generated, and the link is to that new ID: 
`[[id:new-id][H2]]`.

Now, with `org-id-link-to-org-use-id' set to `inherit`, "H2" is not modified, 
and the resulting link is `[[id:abc::*H2][H2]]`, which will still take you to 
the same place as long as the sub-heading is unique within the parent heading 
with an ID.

As an example, I find this useful in situations like this:

#+begin_example
* Project 1
:PROPERTIES:
:ID: project-1
:END:

** <2023-07-01> Meeting A
** <2023-07-08> Meeting B
** <2023-07-15> Meeting C
#+end_example

... so that I can link to specific meetings without needing every one to have 
its own org ID.

Feedback on the patch welcome. If you would like to merge this I will (I 
assume) need to sort out FSF copyright assignment and update ORG-NEWS and the 
manual.

Best
Rick

0001-lisp-org-id.el-Allow-using-a-parent-s-existing-id.patch
Description: Binary data


Re: [HELP] Fwd: Org format as a new standard source format for GNU manuals

2022-10-05 Thread Rick Lupton
Hi Ihor and all,

I wonder if you have seen Pollen’s approach to this? 
https://docs.racket-lang.org/pollen/pollen-command-syntax.html

There are two separate ideas used there which seemed related to this 
discussion. I’m not sure if they are useful in the org context.  

1. The use of a special character (◊ by default) which introduces a 
command/inline special block. I don’t know if this would be worth considering 
as an alternative to @ (which also seems reasonable) to avoid ambiguity with 
other syntax. As the link above discusses it’s harder to type but there are 
solutions. 

2. Making it easy to define custom functions that produce org syntax. A bit 
like perhaps Max's suggestion to use source blocks, but instead of writing 
AST-like structure directly in the document where you want it, you can call a 
previously defined function to build it. This is similar to org macros but I’m 
not sure if they are so flexible as a lisp function. There is also the option 
to choose between passing arguments as lisp (in [ ]) or as markup (in { })

On Tue, 4 Oct 2022, at 9:28 PM, Juan Manuel Macías wrote:
> Ihor Radchenko writes:
>
>> If I were to choose an alternative symbol other than "_", I'd choose
>> "@":
>>
>> @name{}
>> @name{{> @name[:key value ...]{}
>>
>> 1. It is similar to Texinfo
>> 2. It does not clash with TeX
>> 3. We already use @ in the inline export snippets.
>
> I like the "@" alternative a lot. And I agree with all three points. It is
> also compact without losing clarity, and does not give the feeling of a
> blank space before, as in the case of "_".
>
> Best regards,
>
> Juan Manuel



Re: [BUG] org-cite does not support techreport, mastersthesis, phdthesis, and conference entries [9.5.2 (release_9.5.2-426-gf6813d @ /usr/share/emacs/site-lisp/org/)]

2022-04-08 Thread Rick Lupton
I also found this recently. The reason is that BibTeX-dialect is set by 
org-cite-basic--parse-bibliography in oc-basic.el based on the file extension. 
Renaming your bibliography file to end in “.bibtex” instead of “.bib” should 
make it be parsed using the bibtex rather than biblatex dialect. 

I did find this surprising, as I’ve always just used “.bib” for both dialects, 
and unless I was missing something it seemed to be incompatible with the bibtex 
program which seemed to expect a .bib extension. 

Generally the cite export is working well, thank you. 

Best
Rick

On Fri, 8 Apr 2022, at 8:10 PM, Olaf Trygve Berglihn wrote:
> Thanks for the info.  I do get that e.g. TechReport is in
> bibtex-entry-alist, but it does not work still.  I also tried to remove
> any ${HOME}/.emacs.d stuff to ensure no interaction.
>
> bibtex-entry-alist is only populated after an explicit call to
> (load-library "bibtex")
> or by first visiting a bibtex buffer.
>
> #+BEGIN_SRC emacs-lisp
> (mapcar 'car bibtex-entry-alist)
> #+END_SRC
>
> #+RESULTS:
> | Article | InProceedings | Conference | InCollection | InBook | 
> Proceedings | Book | Booklet | PhdThesis | MastersThesis | TechReport | 
> Manual | Unpublished | Misc |
>
>
> bibtex-dialect is a variable defined in ‘bibtex.el’.
> Its value is ‘BibTeX’
>
> Unfortunately, my elisp and org knowledge is limited, so I'm currently
> not able to provide more details, trace, or debugging info without some
> guidance.
>
> Thanks for your help.
>
> -Olaf
>
> -- 
> Olaf Trygve Berglihn 



Re: [O] Bug: Extra spaces in babel #+call arguments [7.9.3d (release_7.9.3d-823-gd06fa4 @ /usr/share/emacs/24.2/site-lisp/org/)

2013-02-01 Thread Rick Lupton
Thomas S. Dye tsd at tsdye.com writes:
 Rick Lupton r.lupton at gmail.com writes:
 
  As a separate question, is it possible to get :results file as the
  default from the original block so it doesn't have to be repeated in each
  #+CALL ?
 
 This should work:
 
 * Appropriate Heading Level
   :PROPERTIES:
   :RESULTS:  file
   :END:
 

Yes, that works fine thanks.

For the other question, it's easy to work around -- as below -- though it'd be
nice not to have to.

#+name: nbimg
#+begin_src sh :var nb= :var tag= :results output file
# get rid of spurious trailing spaces
fn=$(echo $nb | sed 's/^ *//g')
nbimage -o auto_images $fn.ipynb $tag
#+end_src

Thanks,
Rick




[O] Bug: Extra spaces in babel #+call arguments [7.9.3d (release_7.9.3d-823-gd06fa4 @ /usr/share/emacs/24.2/site-lisp/org/)

2013-01-30 Thread Rick Lupton
Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

 http://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org-mode mailing list.


I am using org-babel to include the latest version of
externally-generated images in my org-mode file, which works well. But
there is a problem with using #+CALL: to include a link to the image:
if the argument to #+CALL includes parentheses, extra spaces are
added. Please see the examples below, run with emacs -Q.

(set up sh execution)
#+begin_src lisp
(org-babel-do-load-languages
 'org-babel-load-languages
 '((sh . t)))
#+end_src

The following block works as expected when run with =C-c C-c=: the link is
just the passed filename plus .txt.

#+name: test
#+begin_src sh :var filename=test james (fred) :results file
echo $filename.txt
#+end_src

#+RESULTS: test
[[file:test james (fred).txt]]

But using call adds an extra space when there are parentheses in the
argument:

#+call: test(filename=test) :results file

#+RESULTS: test(filename=test):results file
[[file:test.txt]]

#+call: test(filename=test (foo)) :results file

#+RESULTS: test(filename=test (foo)) :results file
[[file:test (foo) .txt]]

This last line shows the bug.

As a separate question, is it possible to get :results file as the
default from the original block so it doesn't have to be repeated in each
#+CALL ?

Thanks,
Rick

Emacs  : GNU Emacs 24.2.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.10)
 of 2012-10-07 on americium, modified by Debian
Package: Org-mode version 7.9.3d (release_7.9.3d-823-gd06fa4 @
/usr/share/emacs/24.2/site-lisp/org/)

current state:
==
(setq
 org-export-preprocess-before-selecting-backend-code-hook
'(org-beamer-select-beamer-code)
 org-tab-first-hook '(org-hide-block-toggle-maybe
  org-src-native-tab-command-maybe
  org-babel-hide-result-toggle-maybe
  org-babel-header-arg-expand)
 org-speed-command-hook '(org-speed-command-default-hook
  org-babel-speed-command-hook)
 org-occur-hook '(org-first-headline-recenter)
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-export-preprocess-before-normalizing-links-hook
'(org-remove-file-link-modifiers)
 org-confirm-shell-link-function 'yes-or-no-p
 org-export-latex-final-hook '(org-beamer-amend-header org-beamer-fix-toc
   org-beamer-auto-fragile-frames
   org-beamer-place-default-actions-for-lists)
 org-export-latex-after-initial-vars-hook '(org-beamer-after-initial-vars)
 org-after-todo-state-change-hook '(org-clock-out-if-current)
 org-src-mode-hook '(org-src-babel-configure-edit-buffer
 org-src-mode-configure-edit-buffer)
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-babel-pre-tangle-hook '(save-buffer)
 org-mode-hook '(#[nil \300\301\302\303\304$\207
   [org-add-hook change-major-mode-hook org-show-block-all
append local]
   5]
 #[nil \300\301\302\303\304$\207
   [org-add-hook change-major-mode-hook
org-babel-show-result-all append local]
   5]
 org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-ctrl-c-ctrl-c-hook '(org-babel-hash-at-point
  org-babel-execute-safely-maybe)
 org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-hide-drawers
  org-cycle-hide-inline-tasks org-cycle-show-empty-lines
  org-optimize-window-after-visibility-change)
 org-export-latex-format-toc-function 'org-export-latex-format-toc-default
 org-export-first-hook '(org-beamer-initialize-open-trackers)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-babel-load-languages '((sh . t))
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 org-descriptive-links nil
 )