[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [O] Feedback on changes to org-id

From: Aaron Ecay
Subject: Re: [O] Feedback on changes to org-id
Date: Wed, 21 Sep 2016 23:28:55 +0100
User-agent: Notmuch/0.22+21~g7e6e23c (https://notmuchmail.org) Emacs/ (x86_64-unknown-linux-gnu)

Hi Nicolas,

Thanks for your feedback.  I am almost done incorporating it into the
patch.  I have only two further questions.

2016ko irailak 3an, Nicolas Goaziou-ek idatzi zuen:


>> 4. A similar issue arises for org-id-find.  I would like it to always
>> return a marker, rather than having an argument switch between a
>> marker and a cons of filename and position.  (There are functions
>> which return the filename or position individually, so returning both
>> as a cons is useless from an API point of view).  There’s no good way
>> to detect the old calling convention, however, so I think I have to
>> introduce a new name.  (My draft patch is written instead with hard
>> breakage, but stability of API is important so I will change
>> that...)
> Please don't make that change. A marker is pointless if the file is not
> currently associated to any buffer. In that case (file-name . postition)
> cons cell is a valuable return value.

The API has the following two functions already:
- org-id-find-file-for: id -> file-name
- org-id-find-id-in-file: id file -> position

Imagine I add to this API org-id-find-marker: id -> marker.  Then I
think we can deprecate (and eventually delete) org-id-find, since all its
uses can be replaced by some combination of the other 3 functions.  (We
could also keep it as a convenience function wrapping the other 3, but
it hardly seems worth it: the marker case just adds the overhead of
another funcall, whereas a significant proportion of the non-marker
calls in the codebase actually only care about the file name, so it is a
waste of effort to calculate the buffer position only to throw it away.)


> You can also remove `org-id-track-globally'. "org-id.el" is useless if
> it is set to nil anyway, since CUSTOM_ID does a better job in this
> case.

I think this would imply writing the ID database to ‘org-id-locations-file’
under certain circumstances without asking/letting the user approve this
action.  Is that OK?  (I am not bothered by it, FWIW).

If it’s not acceptable, perhaps this variable should be replaced by a
new defcustom ‘org-id-write-database’ which would control only the
writing of the DB to disk (but unlike the existing implementation would
not turn off the ID tracking code paths within the emacs session).


Aaron Ecay

reply via email to

[Prev in Thread] Current Thread [Next in Thread]