search

emacs-citar / citar-org-roam

citar/org-roam integration
GNU General Public License v3.0
101 stars 9 forks source link

feat(capture): make info plist configurable #33

Closed bdarcus closed 1 year ago

bdarcus commented 1 year ago

Add citar-org-roam--make-info-plist function and citar-org-roam-template-fields defcustom to allow configurable creation of org-roam info plists.

@trembel followup here; here's what the results currently look like.

ELISP> (citar-org-roam--make-info-plist "toly2017")
(:citar-title "Brexit, Global Cities, and the Future of World Order" :citar-author "Toly, Noah" :citar-date "2017" :citar-pages "142–149" :citar-type "article")

Not tested extensively, but does this make sense? Certainly making it configurable is, but the org-roam title?

I may be missing some nuance on capture templates. If I can remove it again and users can still configure the note title in the capture template, that may be better?

But I don't think people will want their note titles set to the reference title.

WDYT?

trembel commented 1 year ago

Looks good to me, I will try it out this afternoon!

trembel commented 1 year ago

When anyway in the template we can access title, author, ...; is it then still necessary to have the citar-org-roam-note-title-template?

bdarcus commented 1 year ago

Actually, I'm going to just merge this. Can always fix little bugs or oversights, but it fixes an oversight in the earlier commit, so ... ;-)

bdarcus commented 1 year ago

Can always fix little bugs or oversights ...

... yeah, there's a bug :-(

Should be easy to fix though.

bdarcus commented 1 year ago

Fixed that bug. Which raises a question @trembel:

In the main code, nil properties are included. Should they be?

I have a local fix to remove them, but not sure the implications.

I'm assuming for now it's better to include the nil properties, since my assumption is org-roam will just ignore them, while if they're omitted, it will prompt for their value.

I suppose a third option is to convert nil values to empty strings.

trembel commented 1 year ago

Fixed that bug. Which raises a question @trembel:

In the main code, nil properties are included. Should they be?

I have a local fix to remove them, but not sure the implications.

I'm assuming for now it's better to include the nil properties, since my assumption is org-roam will just ignore them, while if they're omitted, it will prompt for their value.

I suppose a third option is to convert nil values to empty strings.

I don't see exactly where you mean. If org-roam asks for them, this is fine; It means "Hey, this entry is not found in the bib, please provide it". If they are nil, and org-roam does not ask, it is also fine; You can see in the org-roam note that something is missing. What never should be, is that org-roam puts "nil" as a string into the note/db

bdarcus commented 1 year ago

I don't see exactly where you mean. If org-roam asks for them, this is fine; It means "Hey, this entry is not found in the bib, please provide it".

No; it means "there's no data for the entry field".

I'm talking about the info plist.

Consider the case of an entry without a title.

Should I leave that out of the capture plist, or should I do:

  1. :citar-title nil (now)
  2. :citar-title ""

Basically, in that case we don't want org-roam to prompt for the title.

I could just test this, but I figured it might be quicker to ask.

Aside: is that plist data stored in the DB?

bdarcus commented 1 year ago

I could just test this, but I figured it might be quicker to ask.

Easy enough to test; I just created a note with a reference with missing data.

No errors; just nothing is printed there.

So I'll leave as is, I guess.