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

[stardiviner]

Gustav Wikström <address@hidden> writes:

> Hi!
>
> org-attach-store-link-p with option t is supposed to store a link to the 
> original location (i.e. the location the file was/is in before it was 
> attached to the node. That was the default before I started working with 
> attachments I believe. Haven't ever used that feature myself but the patch 
> you provide would change the functionality which I don't think is correct. It 
> would also not match the documentation any longer.
>
> See the documentation for the customization parameter:
>
> #+begin_src emacs-lisp
>   (defcustom org-attach-store-link-p nil
>     "Non-nil means store a link to a file when attaching it."
>     :group 'org-attach
>     :version "24.1"
>     :type '(choice
>             (const :tag "Don't store link" nil)
>             (const :tag "Link to origin location" t)
>             (const :tag "Link to the attach-dir location" attached)))
> #+end_src
>
> Regards
> Gustav

I've been used this functionality for a long time, I always store the link after
attached file. Because the old path is gone.

For example, I have a file in =~/Downloads/kk.png=, then I attached it under a
node, then the file moved to =data/images/kk.png=. The original file
=~/Downloads/kk.png= is gone, does not exist, because I use =mv= method. If 
still
link to original location, so the link file does not exist. That's why I found
it inconsistent.

Here is the commit which might affected this behavior if I guess right.

#+begin_src diff
26ace9004 origin/master Make org-attach store links using attachment-links
modified   lisp/org-attach.el
@@ -487,33 +479,37 @@
 (defun org-attach-attach (file &optional visit-dir method)
   "Move/copy/link FILE into the attachment directory of the current outline 
node.
 If VISIT-DIR is non-nil, visit the directory with dired.
 METHOD may be `cp', `mv', `ln', `lns' or `url' default taken from
 `org-attach-method'."
   (interactive
    (list
     (read-file-name "File to keep as an attachment:"
                     (or (progn
                           (require 'dired-aux)
                           (dired-dwim-target-directory))
                         default-directory))
     current-prefix-arg
     nil))
   (setq method (or method org-attach-method))
   (let ((basename (file-name-nondirectory file)))
     (let* ((attach-dir (org-attach-dir 'get-create))
            (fname (expand-file-name basename attach-dir)))
       (cond
        ((eq method 'mv) (rename-file file fname))
        ((eq method 'cp) (copy-file file fname))
        ((eq method 'ln) (add-name-to-file file fname))
        ((eq method 'lns) (make-symbolic-link file fname))
        ((eq method 'url) (url-copy-file file fname)))
       (run-hook-with-args 'org-attach-after-change-hook attach-dir)
       (org-attach-tag)
       (cond ((eq org-attach-store-link-p 'attached)
-             (org-attach-store-link fname))
+            (push (list (concat "attachment:" (file-name-nondirectory fname))
+                        (file-name-nondirectory fname))
+                  org-stored-links))
             ((eq org-attach-store-link-p t)
-             (org-attach-store-link file)))
+             (push (list (concat "file:" file)
+                        (file-name-nondirectory file))
+                  org-stored-links)))
       (if visit-dir
           (dired attach-dir)
         (message "File %S is now an attachment." basename)))))
#+end_src

And in the commit "26ace9004260a056acef58a7c1c80222bc9587c9":

#+begin_src diff
modified   lisp/org-attach.el
@@ -457,14 +457,6 @@ DIR-property exists (that is different than the unset 
one)."
   "Turn the autotag off."
   (org-attach-tag 'off))
 
-(defun org-attach-store-link (file)
-  "Add a link to `org-stored-link' when attaching a file.
-Only do this when `org-attach-store-link-p' is non-nil."
-  (setq org-stored-links
-       (cons (list (org-attach-expand-link file)
-                   (file-name-nondirectory file))
-             org-stored-links)))
-
 (defun org-attach-url (url)
   (interactive "MURL of the file to attach: \n")
   (let ((org-attach-method 'url))
@@ -511,9 +503,13 @@ METHOD may be `cp', `mv', `ln', `lns' or `url' default 
taken from
       (run-hook-with-args 'org-attach-after-change-hook attach-dir)
       (org-attach-tag)
       (cond ((eq org-attach-store-link-p 'attached)
-             (org-attach-store-link fname))
+            (push (list (concat "attachment:" (file-name-nondirectory fname))
+                        (file-name-nondirectory fname))
+                  org-stored-links))
             ((eq org-attach-store-link-p t)
-             (org-attach-store-link file)))
+             (push (list (concat "file:" file)
+                        (file-name-nondirectory file))
+                  org-stored-links)))
       (if visit-dir
           (dired attach-dir)
         (message "File %S is now an attachment." basename)))))
@@ -642,12 +638,6 @@ See `org-attach-open'."
 Basically, this adds the path to the attachment directory."
   (expand-file-name file (org-attach-dir)))
 
-(defun org-attach-expand-link (file)
-  "Return a file link pointing to the current entry's attachment file FILE.
-Basically, this adds the path to the attachment directory, and a \"file:\"
-prefix."
-  (concat "file:" (org-attach-expand file)))
-
 (org-link-set-parameters "attachment"
                          :follow #'org-attach-open-link
                          :export #'org-attach-export-link
#+end_src

>From the source code, it seems indeed still original code logic.

Then I checked out my Emacs init git log, confirmed it is ~'attached~ option 
value
at first time. So this is my mistake.

Because this new ~attachment:~ link type has potential issues like on exporting.
So I decided to still use old ~file:~ link type. So I thought should set
~org-attach-store-link-p~ to ~t~. It indeed use ~file:~ link type instead of
~attachment:~ now. But the store link behavior affected. 

So the problem is how can I use both ~file:~ link type for attachments and use
this new attached store link?

>
>> -----Original Message-----
>> From: stardiviner <address@hidden>
>> Sent: den 18 januari 2020 15:56
>> To: Gustav Wikström <address@hidden>
>> Cc: address@hidden; address@hidden
>> Subject: Re: [O] FW: [RFC] Link-type for attachments, more attach options
>> 
>> 
>> stardiviner <address@hidden> writes:
>> 
>> I finally figured out why the link always failed. Because it use wrong
>> variable which is old filename path. I attached a patch.
>> 
>> > Gustav Wikström <address@hidden> writes:
>> >
>> >> Hi,
>> >>
>> >>> -----Original Message-----
>> >>> From: stardiviner <address@hidden>
>> >>> Sent: den 15 januari 2020 07:21
>> >>> To: Gustav Wikström <address@hidden>
>> >>> Cc: address@hidden; address@hidden
>> >>> Subject: Re: [O] FW: [RFC] Link-type for attachments, more attach
>> >>> options
>> >>>
>> >>> [...]
>> >>>
>> >>> >> I found when I set option ~(setq org-attach-store-link-p t)~.
>> >>> >> Then attach a file, store file link with =[C-c C-l]=. The stored
>> >>> >> link. I open this link got error "No such file: ....". I tested
>> >>> >> this with minimal Emacs config. confirmed this problem.
>> >>> >>
>> >>> >
>> >>> > I cannot reproduce this. In my try with a minimal Emacs (emacs -q)
>> >>> > and
>> >>> with only that single customization it works for me. I'm testing it
>> >>> in linux. A wild guess.. Could it be that you used the move
>> >>> operation instead of the copy operation when attaching the file?
>> >>> >
>> >>> > Regards
>> >>> > Gustav
>> >>>
>> >>> Did you reproduce this issue with =emacs -q= ? That is a built-in
>> >>> Org Mode version which does not contains the latest version =org-
>> attach.el=.
>> >>>
>> >>> Here is my minimal Emacs config:
>> >>>
>> >>> [...]
>> >>>
>> >>> ;;==================================================================
>> >>> ======
>> >>> ======
>> >>> ;;; Here is org-attach.el customization
>> >>>
>> >>> (require 'org-attach)
>> >>>
>> >>> ;; store link auto with `org-store-link' using `file:' link type or
>> >>> `attachment:' link type.
>> >>> (setq org-attach-store-link-p 'attached) (setq
>> >>> org-attach-dir-relative t) (setq org-attach-preferred-new-method
>> >>> 'ask) #+end_src
>> >>>
>> >>> #+begin_src sh :eval no
>> >>> emacs -q -l '~/.config/emacs/minimal-init.el'
>> >>> #+end_src
>> >>
>> >
>> >> Hmm, in the first mail you said that you set org-attach-store-link-p
>> >> to t, but in your config it says 'attached.
>> >
>> > Sorry about this.
>> >
>> >> I've tried with a minimal config as well (using emacs -q because I
>> >> build the newest org mode version into the emacs
>> >> folder) and can only reproduce your issue when using the attached
>> >> option for org-attach-store-link-p and then inserting that link with
>> >> C-c C-l /in another heading/. Pasting the link in another heading is
>> >> expected to break since the attachment link is context dependent (i.e.
>> requires an attachment folder).
>> >> Makes sense? If I'm still misunderstanding your use-case, would you
>> >> care to describe the steps to reproduce it more in detail?
>> >
>> > After updated commit, don't know why, but all links worked again. I'm
>> > not good at expressing thanks, but you got all my thanks on this. :)
>> >
>> >> Regards
>> >> Gustav
>> >>
>> 
>> --
>> [ stardiviner ]
>>        I try to make every word tell the meaning what I want to express.
>> 
>>        Blog: https://stardiviner.github.io/
>>        IRC(freenode): stardiviner, Matrix: stardiviner
>>        GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
>> 


-- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3