fuxialexander / org-pdftools

A custom org link type for pdf-tools
GNU General Public License v3.0
335 stars 36 forks source link

`org-pdftools-complete-link` produces empty links #31

Closed mohkale closed 4 years ago

mohkale commented 4 years ago

I just found out all the links I made yesterday were unusable :cry:.

Here's the current body of the function.

(defun org-pdftools-complete-link (&optional arg)
  "Use the existing file name completion for file.
Links to get the file name, then ask the user for the page number
and append it. ARG is passed to `org-link-complete-file'."
  (concat
   (replace-regexp-in-string
    "^file:"
    org-pdftools-link-prefix ":"
    (org-link-complete-file arg))
   "::"
   (read-from-minibuffer
    "Page:"
    "1")))

the org-pdftools-link-prefix ":" part should be outside of the replace regexp in string concatenated together as (concat org-pdftools-link-prefix ":"). as it stands, this always outputs ":::1".

fuxialexander commented 4 years ago

Ah...sorry for the loss... Also thanks for the catch!

fuxialexander commented 4 years ago

fixed in https://github.com/fuxialexander/org-pdftools/commit/df6a7da80405decdda78f8b6cdc248ebcfad857b

mohkale commented 4 years ago

Thnx, great work as always 😄.

mohkale commented 4 years ago

Wait, I don't think I was clear enough. That's my bad. You've concatenated both 2nd and 3rd argument to replace-regexp-in-string which means it's not being passed enough arguments and ends up throwing an error.

Here's a diff of how it should look.

  (concat
   (replace-regexp-in-string
    "^file:"
-   org-pdftools-link-prefix ":"
+   (concat
+    org-pdftools-link-prefix ":")
    (org-link-complete-file arg))
  "::"
  (read-from-minibuffer
   "Page:"
fuxialexander commented 4 years ago

Hope I get it right this time 😂

mohkale commented 4 years ago

yep, seems to be working again. :+1:.