Re: Feedback on changes to org-id

[Aaron Ecay <aaronecay@gmail.com>]

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.)

WDYT?

> 
> 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).

TIA,

-- 
Aaron Ecay