Adding key column to db #547

goktug97 · 2020-05-01T12:29:18Z

Refer #540
The key column is added to the refs table to be able to properly link references in the graph view.

TimQuelch · 2020-05-02T01:20:28Z

org-roam-db.el

-   [:insert :into refs
-    :values $v1]
-   (list (vector ref file))))
+  (let ((key (elt (split-string ref ":") 1)))


Won't this run into trouble with other types of links in the ROAM_KEY? e.g. https://example.com will get stripped to //example.com

Maybe we should check if the prefix is in org-ref-cite-types before splitting, and then strip off that prefix. Otherwise leave the key untouched.

Another issue this might run into is if a citekey has a ':' in it, this will strip off the stuff afterwards (after a quick search it appears that ':' is allowed in a bibtex citekey). e.g. in a pathological case: cite:key:withcolon would get stripped to key instead of key:withcolon, or key:withcolon would get stripped to just withcolon.

Suggested change

(let ((key (elt (split-string ref ":") 1)))

(let ((key ref))

(dolist (prefix org-ref-cite-types)

(setq key (string-remove-prefix (concat prefix ":") key)))

TimQuelch · 2020-05-02T01:26:46Z

org-roam-db.el

-            (when-let ((ref (org-roam--extract-ref)))
-              (setq all-refs (cons (vector ref file) all-refs))))
+            (when-let* ((ref (org-roam--extract-ref))
+                        (key (elt (split-string ref ":") 1)))


Same issue as above

TimQuelch · 2020-05-02T05:33:38Z

org-roam-db.el

-   [:insert :into refs
-    :values $v1]
-   (list (vector ref file))))
+  (let ((key (elt (split-string ref ":") 1)))


Suggested change

(let ((key (elt (split-string ref ":") 1)))

(let ((key ref))

(dolist (prefix org-ref-cite-types)

(setq key (string-remove-prefix (concat prefix ":") key)))

jethrokuan

Instead of storing the citekey as key, let's have a db structure as follows:

| ref                | file            | type      | (original key)     |
|--------------------+-----------------+-----------+--------------------|
| https://google.com | /path/to/f1.org | "website" | https://google.com |
| ahrens2017         | /path/to/f2.org | "cite"    | cite:ahrens2017    |
| foo                | /path/to/f3.org | "roam"    | foo                |

jethrokuan · 2020-05-02T07:00:34Z

org-roam-db.el

@@ -133,7 +133,8 @@ SQL can be either the emacsql vector representation, or a string."

    (refs
     [(ref :unique :not-null)
-      (file :not-null)])))
+      (file :not-null)
+      (key :not-null)])))


Let's call this type instead, same as the links table. Key will then take on values:

cite if the prefix is an org-ref cite link (as per org-ref-cite-types)

website if it's a URL (optional, nothing is being done with this)

`roam' otherwise.

Wouldn't this break setups where the ROAM_KEY is just the citekey?

e.g. ROAM_KEY: ahrens2017 would be detected as a roam type rather than a cite type.

having the key (or whatever column name is chosen) just be the original ROAM_KEY stripped of any org-ref-cite-types prefixes should allow both cite:ahrens2017 and ahrens2017 to work

I think it's fine to keep the cite: in ROAM_KEY as a way to identify bibliographic notes. It comes at very little cost to us, and I agree with @jethrokuan that it helps with entry if you're not using org-roam-bibtex to generate those files.

This stems from the desire to have different types of keys co-exists with just one file-metadata tag. Otherwise, another option which wouldn't be as elegant would be to have different file-metadata tags like ROAM_CITEKEY, ROAM_URLKEY, etc.

Fair enough. I can see the logic for future extendability. ROAM_CITEKEY and ROAM_URLKEY does not sound like a nice solution IMO.

I might submit a quick PR to fix the backlinks buffer not catering for all cite types

goktug97 · 2020-05-02T09:44:12Z

Let's call this type instead, same as the links table. Key will then take on values:
1. `cite` if the prefix is an org-ref cite link (as per `org-ref-cite-types`)

2. `website` if it's a URL (optional, nothing is being done with this)

3. `roam' otherwise.

Maybe something like this?

(defun org-roam-db--insert-ref (file ref)
  "Insert REF for FILE into the Org-roam cache."
  (let* ((url (url-generic-parse-url ref))
         (type (url-type url)))
    (cond ((cl-find type org-ref-cite-types :test #'string=)
           (setq ref (string-remove-prefix (concat type ":")))
           (setq type "cite"))
          ((cl-member type '("http" "https") :test #'string=)
           (setq type "website"))
          (t
           (setq type "roam")))
    (org-roam-db-query
     [:insert :into refs :values $v1]
     (list (vector ref file type)))))

bdarcus · 2020-05-02T09:55:11Z

I'm not following the details here, but did just want to mention somewhere that native org citation support is finally moving forward, and I expect to be merged to master "soon."

TimQuelch · 2020-05-02T10:01:46Z

Maybe something like this?

I don't think that cl-find will work correctly.. parencite:key will match to cite. You'll then try to remove the prefix cite: which does not exist. Therefore ref will remain parencite:key

Edit: Actually I may have misunderstood cl-find

TimQuelch · 2020-05-02T11:01:36Z

@goktug97 how about something like this? (untested)

(defun org-roam-db--insert-ref (file ref)
  "Insert REF for FILE into the Org-roam cache."
  (let* ((cite-prefix (seq-find
                       (lambda (prefix) (s-prefix? ref))
                       (-map (lambda (type) (concat type ":"))
                             org-ref-cite-types)))
         (is-website (seq-some
                      (lambda (prefix) (s-prefix? ref))
                      '("http" "https")))
         (type (cond (cite-prefix "cite")
                     (is-website "website")
                     (t "roam")))
         (key (cond ((string= "cite" type) (s-chop-prefix cite-prefix ref))
                    (t ref))))
    (org-roam-db-query
     [:insert :into refs :values $v1]
     (list (vector key file type)))))

goktug97 · 2020-05-02T12:05:35Z

@goktug97 how about something like this? (untested)

(defun org-roam-db--insert-ref (file ref)
  "Insert REF for FILE into the Org-roam cache."
  (let* ((cite-prefix (seq-find
                       (lambda (prefix) (s-prefix? ref))
                       (-map (lambda (type) (concat type ":"))
                             org-ref-cite-types)))
         (is-website (seq-some
                      (lambda (prefix) (s-prefix? ref))
                      '("http" "https")))
         (type (cond (cite-prefix "cite")
                     (is-website "website")
                     (t "roam")))
         (key (cond ((string= "cite" type) (s-chop-prefix cite-prefix ref))
                    (t ref))))
    (org-roam-db-query
     [:insert :into refs :values $v1]
     (list (vector key file type)))))

It works perfectly but prefix was missing in s-prefix? so I added that.

(defun org-roam-db--insert-ref (file ref)
  "Insert REF for FILE into the Org-roam cache."
  (let* ((cite-prefix (seq-find
                       (lambda (prefix) (s-prefix? prefix ref))
                       (-map (lambda (type) (concat type ":"))
                             org-ref-cite-types)))
         (is-website (seq-some
                      (lambda (prefix) (s-prefix? prefix ref))
                      '("http" "https")))
         (type (cond (cite-prefix "cite")
                     (is-website "website")
                     (t "roam")))
         (key (cond ((string= "cite" type) (s-chop-prefix cite-prefix ref))
                    (t ref))))
    (org-roam-db-query
     [:insert :into refs :values $v1]
     (list (vector key file type)))))

To test it;

(dolist (ref '("cite:test" "https://www.github.com" "file:///home/user"))
 (let* ((cite-prefix (seq-find
                      (lambda (prefix) (s-prefix? prefix ref))
                      (-map (lambda (type) (concat type ":"))
                            org-ref-cite-types)))
        (is-website (seq-some
                     (lambda (prefix) (s-prefix? prefix ref))
                     '("http" "https")))
        (type (cond (cite-prefix "cite")
                    (is-website "website")
                    (t "roam")))
        (key (cond ((string= "cite" type) (s-chop-prefix cite-prefix ref))
                   (t ref))))
   (print type)))

and it prints

"cite"

"website"

"roam"
nil

TimQuelch · 2020-05-02T12:39:43Z

It works perfectly but prefix was missing in s-prefix? so I added that.

oops my bad.

Yeah I just tested too and it seems to work

goktug97 · 2020-05-02T13:27:25Z

Okay if we are choosing this route what are we going to do here.
https://github.com/jethrokuan/org-roam/blob/e698ed7f5378106da8a8fec4537658392157657c/org-roam-graph.el#L175-L176

EDIT: Of course it should be ref instead of key :D

goktug97 · 2020-05-02T15:26:54Z

I added preview functionality to the org-roam-server,
as soon as we are done with this PR, I will push the changes to the repo and open a pull request for the MELPA

EDIT: Pushed to Github because it seems there is no change needed to org-roam-server after this PR is pushed.

goktug97 · 2020-05-02T16:22:28Z

The latest commits are not working. I thought they are working but after resetting the emacs I realized the new functions weren't properly loaded in the previous session.

EDIT: It is fixed and everything works properly.

… to changelog

goktug97 · 2020-05-02T19:39:54Z

I don't know how MELPA works but should we also update this to 1.1.2

zaeph · 2020-05-02T19:41:59Z

I don't know how MELPA works but should we also update this to 1.1.2

I don't think this plays a rôle in MELPA, but yes, this is something that we should update.

Edit: Oh, I thought we'd made it to 1.1.2. It seems that we're still on 1.1.0, though, so what's the problem?

goktug97 · 2020-05-02T19:51:45Z

I should have appended the changelog to 1.1.1 instead of creating a new version? If that's true, do you recommend ammend or a new commit?

zaeph · 2020-05-04T11:19:38Z

Allow edits by maintainers was on but I merged the PR

Now I know. It's because you've been making your changes on master instead of on a feature-branch.

jethrokuan · 2020-05-04T11:21:12Z

CI fails without #562 merged, but #562 causes CI to fail because of compilation errors in org-ref, which I'm tracking here: jkitchin/org-ref#735

zaeph · 2020-05-04T11:24:33Z

CI fails without #562 merged, but #562 causes CI to fail because of compilation errors in org-ref, which I'm tracking here: jkitchin/org-ref#735

Thanks for the work. I'm not sure if it's needed for bibtex-completion to require Emacs 26.1.

Edit: Oh, just realised you meant this for org-ref, and that it's probably just a checking they need to change for their Travis.

goktug97 · 2020-05-04T12:02:39Z

289a792 Docstring update by @jethrokuan
7d1fc3e Same docstring update by @goktug97
Sorry it seems you've already made the changes

TimQuelch · 2020-05-04T12:20:17Z

Woooo tests passing. That red x has been haunting this PR for a long time

The format of `INFO` argument of `org-roam-find` has been simplified, thanks to the split of `type` and `ref` in the db permitted by org-roam#547. The non-interactive behaviour of `org-roam-find-ref` has also been isolated into `org-roam--find-ref`. Even if it makes us repeat ourselves a bit, it avoids having to nil the interactive argument just to get the non-interactive us, which I think is better syntax.

* (feat): simplify format of `INFO` for find-ref The format of the `INFO` argument of `org-roam-find` has been simplified, thanks to the split of `type` and `ref` in the db permitted by #547. The non-interactive behaviour of `org-roam-find-ref` has also been isolated into `org-roam--find-ref`. Even if it makes us repeat ourselves a bit, it avoids having to nil the interactive argument just to get the non-interactive us, which I think is better syntax. * (feat): implement get-ref-path-completions filtering

… to changelog

… in extract-ref

…rg-roam-db--update-maybe`

goktug97 added 6 commits May 1, 2020 12:03

add citation key as column to refs

7797dde

fix unwanted ref

8e6cafa

ref to key

97506ba

Updating fork to master

d7c278d

use key instead of ref

8e3030b

Merge branch 'master' into master

cb4fb35

TimQuelch suggested changes May 2, 2020

View reviewed changes

TimQuelch mentioned this pull request May 2, 2020

Stop formatting citekeys as Org-ref links in ROAM_KEY org-roam/org-roam-bibtex#22

Closed

TimQuelch suggested changes May 2, 2020

View reviewed changes

jethrokuan reviewed May 2, 2020

View reviewed changes

zaeph mentioned this pull request May 2, 2020

org-roam-bibtex-note-actions first commit org-roam/org-roam-bibtex#21

Merged

TimQuelch mentioned this pull request May 2, 2020

Generate cite backlinks using all org ref cite types #551

Merged

goktug97 added 4 commits May 2, 2020 19:14

add type column to refs

9022918

Merge branch 'master' of https://github.com/goktug97/org-roam

dc295d0

Merge branch 'master' into master

ae9a5a7

Merge branch 'master' of https://github.com/goktug97/org-roam

8628c8a

goktug97 added 2 commits May 2, 2020 20:14

fix and cleanup

68a91e0

[FIX org-roam#547] Add missing docstrings to new functions and append…

8e9662b

… to changelog

jethrokuan and others added 2 commits May 4, 2020 19:00

Update docstring for org-roam--cite-prefix

289a792

Improve docstring

4def9c6

goktug97 added 2 commits May 4, 2020 14:54

Better docstring for org-roam--cite-prefix

7d1fc3e

fix merge conflict

d3e251a

goktug97 added 2 commits May 4, 2020 15:10

Update tests for database version 3 [org-roam#547]

6a45eea

Specify modes as symbol in docstrings

d104c48

goktug97 requested a review from jethrokuan May 4, 2020 12:19

jethrokuan approved these changes May 4, 2020

View reviewed changes

jethrokuan merged commit 11d239d into org-roam:master May 4, 2020

goktug97 mentioned this pull request May 4, 2020

Citations are not linked in the graph view #540

Closed

zaeph mentioned this pull request May 4, 2020

org-roam-find-ref cannot find the bibliographic note anymore org-roam/org-roam-bibtex#32

Closed

zaeph mentioned this pull request May 4, 2020

(feat): simplify format of INFO for find-ref and add FILTER #567

Merged

goktug97 mentioned this pull request May 11, 2020

error: (emacsql-error no such column: refs.type) org-roam/org-roam-server#6

Closed

goktug97 mentioned this pull request May 19, 2020

Graphs are not shown (blank page with 5 buttons only) org-roam/org-roam-server#12

Closed

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[FIX org-roam#547] Add missing docstrings to new functions and append…

48f0f9a

… to changelog

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[FIX org-roam#547] Make suggested changes

32b7ed3

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[FIX org-roam#547] Add type check to graph build and fix return value…

ca995bb

… in extract-ref

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[FIX org-roam#547] Update database if version is old

3244504

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[FIX org-roam#547] Use new force attribute while updating the version

72235e5

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[org-roam#547] Deprecate org-roam-db--maybe-update, in favour of `o…

a666c83

…rg-roam-db--update-maybe`

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[org-roam#547] Add more information about the database version upgrade

d457ed3

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

[org-roam#547] Make changes to pass CI tests.

ceca76e

goktug97 added a commit to goktug97/org-roam that referenced this pull request Jun 4, 2020

Update tests for database version 3 [org-roam#547]

a3860b2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding key column to db #547

Adding key column to db #547

goktug97 commented May 1, 2020

TimQuelch May 2, 2020

TimQuelch May 2, 2020

TimQuelch May 2, 2020

TimQuelch May 2, 2020

jethrokuan left a comment

jethrokuan May 2, 2020

TimQuelch May 2, 2020

zaeph May 2, 2020

TimQuelch May 2, 2020

goktug97 commented May 2, 2020 •

edited

bdarcus commented May 2, 2020

TimQuelch commented May 2, 2020 •

edited

TimQuelch commented May 2, 2020 •

edited

goktug97 commented May 2, 2020

TimQuelch commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

goktug97 commented May 2, 2020

zaeph commented May 2, 2020 •

edited

goktug97 commented May 2, 2020

zaeph commented May 4, 2020 •

edited

jethrokuan commented May 4, 2020

zaeph commented May 4, 2020 •

edited

goktug97 commented May 4, 2020 •

edited

TimQuelch commented May 4, 2020

-  (let ((key (elt (split-string ref ":") 1)))
+  (let ((key ref))
+      (dolist (prefix org-ref-cite-types)
+        (setq key (string-remove-prefix (concat prefix ":") key)))

Adding key column to db #547

Adding key column to db #547

Conversation

goktug97 commented May 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jethrokuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goktug97 commented May 2, 2020 • edited

bdarcus commented May 2, 2020

TimQuelch commented May 2, 2020 • edited

TimQuelch commented May 2, 2020 • edited

goktug97 commented May 2, 2020

TimQuelch commented May 2, 2020 • edited

goktug97 commented May 2, 2020 • edited

goktug97 commented May 2, 2020 • edited

goktug97 commented May 2, 2020 • edited

goktug97 commented May 2, 2020

zaeph commented May 2, 2020 • edited

goktug97 commented May 2, 2020

zaeph commented May 4, 2020 • edited

jethrokuan commented May 4, 2020

zaeph commented May 4, 2020 • edited

goktug97 commented May 4, 2020 • edited

TimQuelch commented May 4, 2020

goktug97 commented May 2, 2020 •

edited

TimQuelch commented May 2, 2020 •

edited

TimQuelch commented May 2, 2020 •

edited

TimQuelch commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

goktug97 commented May 2, 2020 •

edited

zaeph commented May 2, 2020 •

edited

zaeph commented May 4, 2020 •

edited

zaeph commented May 4, 2020 •

edited

goktug97 commented May 4, 2020 •

edited