Hello, Gustav Wikström <gus...@whil.se> writes:
> I’ve attached a patch with some suggested additions to org-attach. > Patch comments below. Please review. Thank you. Some comments follow. > * Add new linktype "attached" for attachments This seem a bit long. Why not "att" links? > * Add further options for ATTACH_DIR > > When working with ATTACH_DIR there are now a couple of new options available: > - org-attach-dir-inherit-by-default What is the difference between this and `org-attach-allow-inheritance'? > - org-attach-dir-create-if-not-exist What is the use-case for this one? It doesn't seem terribly useful at first glance. > - org-attach-dir-relative I'm not sure to understand this one. > > +This list shows the full set of built-in external link types: > +| http | web | > +| https | secure web | > +| doi | DOI for electronic resources | > +| file | file-links | > +| file+sys | file-links forced to open via OS | > +| file+emacs | file-links forced to open via emacs | > +| attached | links to attachments for nodes | > +| docview | doc-view mode | > +| id | Link to heading by id | > +| news | Usenet link | > +| mailto | mail link | > +| mhe | MH-E folder link | > +| rmail | Rmail link | > +| gnus | Gnus link | > +| bbdb | BBDB link | > +| irc | IRC link | > +| info | Info link | > +| shell | shell command | > +| elisp | interactive elisp command link | > + > +The following list shows examples for each link type. > + > +| =http://www.astro.uva.nl/=dominik= | on the web > | > +| =doi:10.1000/182= | DOI for an electronic resource > | > +| =file:/home/dominik/images/jupiter.jpg= | file, absolute path > | > +| =/home/dominik/images/jupiter.jpg= | same as above > | > +| =file:papers/last.pdf= | file, relative path > | > +| =./papers/last.pdf= | same as above > | > +| =file:/ssh:me@some.where:papers/last.pdf= | file, path on remote machine > | > +| =/ssh:me@some.where:papers/last.pdf= | same as above > | > +| =file:sometextfile::NNN= | file, jump to line number > | > +| =file:projects.org= | another Org file > | > +| =file:projects.org::some words= | text search in Org file[fn:28] > | > +| =file:projects.org::*task title= | heading search in Org file > | > +| =file+sys:/path/to/file= | open via OS, like double-click > | > +| =file+emacs:/path/to/file= | force opening by Emacs > | > +| =attached:projects.org= | file in folder attached to > headline | > +| =attached:projects.org::some words= | text search in attached file > | > +| =docview:papers/last.pdf::NNN= | open in doc-view mode at page > | > +| =id:B7423F4D-2E8A-471B-8810-C40F074717E9= | link to heading by ID > | > +| =news:comp.emacs= | Usenet link > | > +| =mailto:ad...@galaxy.net= | mail link > | > +| =mhe:folder= | MH-E folder link > | > +| =mhe:folder#id= | MH-E message link > | > +| =rmail:folder= | Rmail folder link > | > +| =rmail:folder#id= | Rmail message link > | > +| =gnus:group= | Gnus group link > | > +| =gnus:group#id= | Gnus article link > | > +| =bbdb:R.*Stallman= | BBDB link (with regexp) > | > +| =irc:/irc.com/#emacs/bob= | IRC link > | > +| =info:org#External links= | Info node link > | > +| =shell:ls *.org= | shell command > | > +| =elisp:org-agenda= | interactive Elisp command > | > +| =elisp:(find-file "Elisp.org")= | Elisp form to evaluate > | I'm not sure to like the change above. It introduces a lot of redundancy. > Here is the syntax of the different ways to attach a search to a file > link, together with explanations for each: > > #+begin_example > -[[file:~/code/main.c::255]] > -[[file:~/xx.org::My Target]] > -[[file:~/xx.org::*My Target]] > -[[file:~/xx.org::#my-custom-id]] > -[[file:~/xx.org::/regexp/]] > + [[file:~/code/main.c::255]] > + [[file:~/xx.org::My Target]] > + [[file:~/xx.org::*My Target]] > + [[file:~/xx.org::#my-custom-id]] > + [[file:~/xx.org::/regexp/]] > + [[attached:~/code/main.c::255]] > #+end_example Please discard this change. > +Another method is /attachments/, which are files located in a > +directory belonging to an outline node. Org uses directories either > +named by the unique ID of each entry, or by the =ATTACH_DIR= property. > +ID-based directories are by default located in the ~data~ directory ... =data/= directory, which ... > +(defcustom org-attach-dir-inherit-by-default nil > + "Defines whether ATTACH_DIR-directories should be inherited by > + subheadings by default when created. Defaults to not being > + inherited." > + :group 'org-attach > + :type 'boolean) First line in a docstring must be a full sentence. Indentation is wrong in the subsequent lines. Also, the `defcustom' is missing keywords, e.g., :package-version and :safe. > +(defcustom org-attach-dir-create-if-not-exists t > + "Choose whether ATTACH_DIR-directories should be created if > +they do not exist since before. Default is to create them." > + :group 'org-attach > + :type 'boolean) > + > +(defcustom org-attach-dir-relative nil > + "Choose whether ATTACH_DIR-directories should be added as > +relative links or not. Defaults to not relative." > + :group 'org-attach > + :type 'boolean) Ditto. > (defcustom org-attach-commit t > "If non-nil commit attachments with git. > This is only done if the Org file is in a git repository." > @@ -280,20 +299,26 @@ Throw an error if we cannot root the directory." > "Set the ATTACH_DIR node property and ask to move files there. > The property defines the directory that is used for attachments > of the entry. When called with `\\[universal-argument]', reset \ > -the directory to > -the default ID based one." > +the directory to the default ID based one. Creates relative links You need to use two spaces to separate sentences. > + (let ((old (org-attach-dir nil)) > + (new > + (progn > + (if arg (org-entry-delete nil "ATTACH_DIR") > + (let* ((attach-dir (read-directory-name > + "Attachment directory: " > + (org-entry-get nil > + "ATTACH_DIR"))) > + (current-dir (file-name-directory (or load-file-name > + buffer-file-name))) Why `load-file-name'? Why not using `default-directory'? > +(defun org-attach-open-link (link &optional in-emacs) > + "LINK is expanded with the attached directory and opened the same > +way as file-links are." You need to expound the arguments in the docstring. > + (file-types-re (format "[][]\\[\\(?:file\\|attached\\|[./~]%s\\)" > (if (not link-abbrevs) "" > (format "\\|\\(?:%s:\\)" > (regexp-opt link-abbrevs)))))) Why is it needed? "attached" link type is already registered, so you don't need to also hard-code it here. > + (when (and (or (equal "file" (org-element-property :type link)) > + (equal "attached" (org-element-property :type > link))) Ditto. > + (let ((file (if (equal "attached" (org-element-property :type > link)) > + (require 'org-attach) > + (org-attach-expand > + (org-link-unescape > + (org-element-property :path link))) > + (expand-file-name > + (org-link-unescape > + (org-element-property :path link)))))) Ditto. Regards, -- Nicolas Goaziou
