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