"Rick Lupton" <m...@ricklupton.name> writes: > Please find attached updated patch which I think addresses all the points > discussed. Let me know if you see any further changes needed.
Thanks! 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 <id:some-id::non-existing-target>, 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. 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 <point here> :PROPERTIES: :CUSTOM_ID: subid :END: When you M-x org-store-link, the stored link has ::*Sub instead of the expected ::#subid 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. More inline comments below. > + #+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. > -(defun org-man-store-link () > +(defun org-man-store-link (&optional _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. > +**** =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 ~org-store-link~ was called interactively. Please also explain that the existing functions are not broken. > +*** ~org-id-store-link~ now adds search strings for precise link targets > + > +This new behaviour can be disabled generally by setting > +~org-id-link-use-context~ to ~nil~, or when storing a specific link by > +passing a prefix argument to ~org-store-link~. universal argument. There are several possible prefix arguments in `org-store-link', but only C-u (universal argument) will give the described effect. Also, won't the behavior be _toggled_ by the universal argument? > +When using this feature, IDs should not include =::=, which is used in > +links to indicate the start of the search string. For backwards > +compability, existing IDs including =::= will still be matched (but > +cannot be used together with precise link targets). Please add an org-lint checker that warns about such IDs and mention this checker in the above. Also, this paragraph belongs to "Breaking changes", not "new and changed options". > +*** New option ~org-id-link-consider-parent-id~ to allow =id:= links to > parent headlines > + > +For =id:= links, when this option is enabled, ~org-store-link~ will > +look for ids from parent/ancestor headlines, if the current headline > +does not have an id. > + > +Combined with the new ability for =id:= links to use search strings > +for precise link targets (when =org-id-link-use-context= is =t=, which > +is the default), this allows linking to specific headlines without > +requiring every headline to have an id property, as long as the > +headline is unique within a subtree that does have an id property. > + > +By giving files top-level id properties, links to headlines in the > +file can be made more robust by using the file id instead of the file > +path. Please, provide an example here as well. > +(defun org-link--try-link-store-functions (interactive?) > + "Try storing external links, prompting if more than one is possible. > + > +Each function returned by `org-store-link-functions' is called in > +turn. If multiple functions return non-nil, prompt for which > +link should be stored. > + > +Return t when a link has been stored in `org-link-store-props'." Please document INTERACTIVE? argument in the docstring. > + (let ((results-alist nil)) > + (dolist (f (org-store-link-functions)) > + (when (condition-case nil > + (funcall f interactive?) > + ;; XXX: The store function used (< Org 9.7) to accept no > + ;; arguments; provide backward compatibility support for > + ;; them. Use FIXME, not XXX. (I have no idea why it is XXX in the existing code). > +(defun org-link-precise-link-target (&optional relative-to) > + "Determine search string and description for storing a link. > + > +If a search string is found, return cons cell (SEARCH-STRING > +. DESC). Otherwise, return nil. > + > +If there is an active region, the contents is used (see > +`org-link--context-from-region'). It is not clear from this sentence whether the contents is used for SEARCH-STRING of DESC. > +In org-mode buffers, if point is at a named element (e.g. a > +source block), the name is used. If within a heading, the current > +heading is used. Please use double space between sentences. > +Optional argument RELATIVE-TO specifies the buffer position where > +the search will start from. If the search target that would be > +returned is already at this location, return nil to avoid > +unnecessary search strings (for example, when using search > +strings to find targets within org-id links)." It is not clear what will happen if RELATIVE-TO is before/after point. > - (let (link cpltxt desc search custom-id agenda-link) ;; description > + (let ((org-link-context-for-files (org-xor org-link-context-for-files > + (equal arg '(4)))) > + link cpltxt desc search custom-id agenda-link) ;; description > (cond > ;; Store a link using an external link type, if any function is > - ;; available. If more than one can generate a link from current > - ;; location, ask which one to use. > + ;; available. If more than one can generate a link from > + ;; current location, ask which one to use. Negate > + ;; `org-context-in-file-links' when given a single prefix arg. The part of the comment about negation, should probably be moved near the let binding of `org-link-context-for-files'. > +For example, given this org file: > + > +* Parent > +:PROPERTIES: > +:ID: abc > +:END: > +** Child 1 > +** Child 2 > + > +With `org-id-link-consider-parent-id' set to t, storing a link > +with point at \"Child 1\" will produce a link \"id:abc\" to > +\"Parent\". This is actually confusing. May we only consider parent when `org-id-link-use-context' is enabled? > -(defun org-id-get (&optional epom create prefix) > +(defun org-id-get (&optional epom create prefix inherit) > "Get the ID property of the entry at EPOM. > EPOM is an element, marker, or buffer position. > If EPOM is nil, refer to the entry at point. > If the entry does not have an ID, the function returns nil. > +If INHERIT is non-nil, parents' IDs are also considered. > However, when CREATE is non-nil, create an ID if none is present already. > PREFIX will be passed through to `org-id-new'. > In any case, the ID of the entry is returned." What about both CREATE and INHERIT being non-nil? > +;;;###autoload > +(defun org-id-store-link-maybe (&optional interactive?) > + "Store a link to the current entry using its ID if enabled. > + > +The value of `org-id-link-to-org-use-id' determines whether an ID > +link should be stored, using `org-id-store-link'. > + > +Assume the function is called interactively if INTERACTIVE? is > +non-nil." > + (interactive "p") Do we really need to make it interactive? -- 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>