Re: [O] FW: [RFC] Link-type for attachments, more attach options

[Nicolas Goaziou]

Hello,

Gustav Wikström <address@hidden> 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:address@hidden:papers/last.pdf= | file, path on remote machine  
>       |
> +| =/ssh:address@hidden: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:address@hidden                 | 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