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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Gustav Wikström <gustav@whil.se> writes:

> +	(if should-get
> +	    (progn (message "Running git annex get \"%s\"." path-relative)
> +		   (call-process "git" nil nil nil "annex" "get" path-relative))
> +	  (error "File %s stored in git annex but it is not available, and was not retrieved"
> +		 path))))))

Nitpick:

    (unless should-get 
     (error "File %S stored in git annex but unavailable" path))
    (message "Running git annex get %S." path-relative)
    (call-process ...)

> +Selective means to respect the inheritance setting in
> +`org-use-property-inheritance'."
>    :group 'org-attach
> +  :type '(choice
> +	  (const :tag "Don't use inheritance" nil)
> +	  (const :tag "Inherit parent node attachments" t)
> +	  (const :tag "Respect org-use-property-inheritance" selective)
> +	  )

Dangling paren spotted.

> +      (setq attachment (or (org-attach-dir)
> +			   (quote  "Can't find an existing attachment-folder")))

You forgot to remove that weird quote. Maybe you meant `error'?

> +    (if attach-dir
> +	(progn (if (not (file-directory-p attach-dir))
> +		   (make-directory attach-dir t))
> +	       attach-dir)
> +      (error "No attachment directory is associated with the current node"))))

Same nitpick as above:

    (unless attach-dir
     (error "No attachment ..."))
    (if (file-directory-p attach-dir) attach-dir
      (make-directory attach-dir))

> +(defun org-attach-dir-from-id (id)
> +  "Creates a path based on `org-attach-id-dir' and ID."
> +  (expand-file-name
> +   (funcall org-attach-id-to-path-function id)
> +   (expand-file-name org-attach-id-dir)))

Creates path -> Return a file name.

> +of the entry.  Creates relative links if `org-attach-dir-relative'
> +is t.

Nitpick:

  is t -> is non-nil.

If tests pass, feel free to apply the patches in master. Thank you!

Regards,

-- 
Nicolas Goaziou