From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: Feedback on changes to org-id Date: Sat, 03 Sep 2016 10:25:25 +0200 Message-ID: <87oa45z8y2.fsf@saiph.selenimh> References: <87fuphzv70.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bg6GZ-0000cH-Ri for emacs-orgmode@gnu.org; Sat, 03 Sep 2016 04:25:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bg6GV-0007a9-GQ for emacs-orgmode@gnu.org; Sat, 03 Sep 2016 04:25:30 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:47350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bg6GV-0007Zy-5b for emacs-orgmode@gnu.org; Sat, 03 Sep 2016 04:25:27 -0400 Received: from saiph.selenimh (unknown [IPv6:2a03:a0a0:0:4301::b3c]) (Authenticated sender: mail@nicolasgoaziou.fr) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 00BBF172097 for ; Sat, 3 Sep 2016 10:25:24 +0200 (CEST) In-Reply-To: <87fuphzv70.fsf@gmail.com> (Aaron Ecay's message of "Sat, 03 Sep 2016 01:24:51 +0100") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Org-mode Hello, Aaron Ecay writes: > It=E2=80=99s an occasional project of mine to try to improve and refactor > aspects of org=E2=80=99s code. That's a very good idea, thanks. BTW, please make sure that deprecation warnings all go into the same location, namely "org-compat.el", instead of scattering them all over the place. > I=E2=80=99ve been going through org-id recently. The > following issues came up that I would appreciate feedback on: > > 1. org-id is not loaded by default; it is supposed to be selected by the > user (see footnote 2 of (info "(org) Handling links")). Yet, other > org libraries rely on it, without requiring it (e.g. ox-icalendar). > Also, at least one library (org-bibtex) reimplements bits of org-id=E2= =80=99s > internals to avoid requiring it explicitly. Is there a good reason > not to make org-id a core library, that is to require it > unconditionally from org.el? The reason is loading speed. That is also why "ox.el" is not loaded by default. Instead, loading mechanism relies on autoloads, i.e., you don't have to explicitly load it, it will be so as soon as you call one of its public functions. I think "org-bibtex" should simply assume "org-id" is loaded and call whatever function is needed. There is no reason to avoid requiring it explicitly. At least, that's the idea. OTOH, I'm not opposed to requiring it explicitly. > 2. I would like to deprecate the org-id-method variable. This allows > choosing different methods of generating random IDs. But one method > is as good as another (they are random*...), so we can just always use > a single method (powered by the elisp =E2=80=98random=E2=80=99 functio= n). Choosing > this allows deprecating several other variables and functions, along > with a soft dependency on the external uuidgen binary. Any > objections to this course of action? OK. > 3. I would like to change the API of the org-id-get function. The > current signature is (&optional pom create prefix). POM > (i.e. position or marker) is a useless argument, because in the > (relatively uncommon) case that callers are interested in a location > other than (point) they can wrap the call in (org-with-point-at > ...). I sympathize with the idea, but the POM argument is consistent with the property API (e.g., org-entry-get'). So, it seems awkward to change one part without touching the others. I suggest to keep it for the time being. > PREFIX is similarly useless (and in fact unused in org=E2=80=99s code = base) > because a caller could let-bind org-id-prefix around the call. The > new signature would be (&optional create reset), which are both > boolean arguments. The question arises of how to make this change. > Options I see: > a. Hard breakage; code using the old calling convention will break. > b. Introduce a new function under a new name, deprecate the old name > c. Try to detect which calling convention is in use. > Options (a) and (b) have drawbacks. I would like to implement > (c) by requiring the create and reset arguments, if given, to have > values 'create and 'reset respectively. The old and new calling > conventions have identical semantics when both arguments are nil, so > that case is not a problem. With the new code, any other value for > these arguments (besides nil and a same-named symbol) would indicate > use of the old convention, and signal an error. Comments? I don't like (c) because it probably means we're not going to let it go anytime soon. Also, it means that (org-id-get t t) is not valid using the new signature. I'd go with (a) after Org 9.0 is released, or (b) right now, e.g., with a new `org-id' function (which is shorter, BTW). > 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=E2=80=99s no g= ood 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. Besides, a function always returning a marker is almost certainly a bad idea. Markers need to be reused, when appropriate, or set to nil. Otherwise, you leave active markers everywhere, which slows down Emacs. IOW, requiring explicitly a marker is a way to think about the future of the marker and leads to better code. > There are other deprecations and renamings as well, but none of them > should break third-party code. The resultant patch shrinks the codebase > by 60-ish lines and eliminates 3 defcustoms...baby steps. 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. >A draft patch is attached to this message; I expect to make further >changes based on feedback I receive, so detailed code review (while >certainly always appreciated!) can be postponed until the conceptual > -(defvar org-id-files nil > - "List of files that contain IDs.") > +(defconst org-id-files nil > + "Use the function instead.") > +(make-obsolete-variable 'org-id-files "use the identically-named > function instead." "Org 9.0") This should go in org-compat.el. The same goes for other occurrences. Regards, --=20 Nicolas Goaziou