From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] Package proposal: EBDB Date: Mon, 14 Aug 2017 19:15:54 -0400 Message-ID: References: <87efsxspgv.fsf@ericabrahamsen.net> <87mv784f1h.fsf@ericabrahamsen.net> <87shgwgtyp.fsf@ericabrahamsen.net> <87shgux6s1.fsf@ericabrahamsen.net> <87mv722l8s.fsf@ericabrahamsen.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1502752660 4295 195.159.176.226 (14 Aug 2017 23:17:40 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 14 Aug 2017 23:17:40 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Aug 15 01:17:36 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dhObu-0000K6-Hn for ged-emacs-devel@m.gmane.org; Tue, 15 Aug 2017 01:17:26 +0200 Original-Received: from localhost ([::1]:36993 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dhOc1-0004hW-22 for ged-emacs-devel@m.gmane.org; Mon, 14 Aug 2017 19:17:33 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:57882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dhOaj-0004ea-W0 for emacs-devel@gnu.org; Mon, 14 Aug 2017 19:16:17 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dhOag-0002U8-PY for emacs-devel@gnu.org; Mon, 14 Aug 2017 19:16:13 -0400 Original-Received: from [195.159.176.226] (port=40760 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dhOag-0002Tr-FR for emacs-devel@gnu.org; Mon, 14 Aug 2017 19:16:10 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1dhOaQ-0003we-Fi for emacs-devel@gnu.org; Tue, 15 Aug 2017 01:15:54 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 160 Original-X-Complaints-To: usenet@blaine.gmane.org Cancel-Lock: sha1:kElEpcPNgv/1VY1YlQ+Xnxp3iXw= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 195.159.176.226 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:217552 Archived-At: > Aha! Thank you, that's what I was missing. It seems to have worked, I > guess I'll know for sure when the package gets built. I just noticed the following issues: - You use oset-default on instance fields. This used to work in earlier EIEIO and still mostly work now thanks to a hack, but it creates a weird/adhoc semantics in terms of interaction with the :initform, so I'd like to get rid of this backward compatibility. - ebdb-vm and ebdb-mu4e will break the compilation of the package if the user doesn't have VM and mu4e installed. The appended patch tries to fix those two, but please take a look at it to make sure it still works correctly (especially the ebdb-vm part is quick&dirty, leaving a lot of warnings when VM is not installed, some of them may be real bugs). > Assuming all goes well, can I push this documentation patch to ELPA? Yes, please do, thank you, Stefan diff --git a/.gitignore b/.gitignore new file mode 100644 index 000000000..f56da6eb7 --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +*.elc +ebdb-autoloads.el +ebdb-pkg.el + diff --git a/ebdb-format.el b/ebdb-format.el index cd8ccc5ae..46726e15f 100644 --- a/ebdb-format.el +++ b/ebdb-format.el @@ -40,7 +40,8 @@ (coding-system :type symbol :initarg :coding-system - :initform nil + ;; "`," is used to trick EIEIO into evaluating the form. + :initform `,buffer-file-coding-system :documentation "The coding system for the formatted file/buffer/stream.") ;; TODO: Provide for "psuedo field classes" like 'primary-mail and @@ -93,8 +94,6 @@ :documentation "Abstract base class for EBDB formatters. Subclass this to produce real formatters.") -(eieio-oset-default 'ebdb-formatter 'coding-system buffer-file-coding-system) - (cl-defmethod ebdb-string ((fmt ebdb-formatter)) (slot-value fmt 'object-name)) diff --git a/ebdb-mu4e.el b/ebdb-mu4e.el index 2e4ee5e20..8d0798391 100644 --- a/ebdb-mu4e.el +++ b/ebdb-mu4e.el @@ -25,9 +25,11 @@ ;;; Code: (require 'ebdb-mua) -(require 'mu4e-view) +(if t (require 'mu4e-view)) ;;Dummy test to `require' only at runtime. ;; Tackle `mu4e-headers-mode' later +(defvar mu4e~view-buffer-name) +(defvar mu4e-view-mode-map) (cl-defmethod ebdb-mua-message-header ((header string) &context (major-mode mu4e-view-mode)) @@ -45,7 +47,7 @@ ;; Why wasn't `ebdb-mua-auto-update' ever hooked in to mu4e? -(add-hook 'mu4e-main-mode-hook 'ebdb-insinuate-mu4e) +(add-hook 'mu4e-main-mode-hook #'ebdb-insinuate-mu4e) (provide 'ebdb-mu4e) ;;; ebdb-mu4e.el ends here diff --git a/ebdb-vm.el b/ebdb-vm.el index 27f5f4c69..00766e10b 100644 --- a/ebdb-vm.el +++ b/ebdb-vm.el @@ -25,6 +25,7 @@ (require 'ebdb-com) (require 'ebdb-mua) +(when t ;;Dummy test to `require' only at runtime. (require 'vm-autoloads) (require 'vm) (require 'vm-motion) @@ -33,7 +34,7 @@ (require 'vm-vars) (require 'vm-macro) (require 'vm-message) -(require 'vm-misc) +(require 'vm-misc)) (defgroup ebdb-mua-vm nil "VM-specific EBDB customizations" @@ -366,9 +367,11 @@ from different senders." ;; the EBDB record of the sender. (lambda (m) (ebdb-mua-summary-mark (vm-su-from m)))))) +;; FIXME: `vm' is required earlier, so (eval-after-load "vm" ...) doesn't make +;; much sense at this point. (eval-after-load "vm" '(ebdb-insinuate-vm)) -(add-hook 'vm-select-message-hook 'ebdb-mua-auto-update) +(add-hook 'vm-select-message-hook #'ebdb-mua-auto-update) (provide 'ebdb-vm) ;;; ebdb-vm.el ends here diff --git a/ebdb.el b/ebdb.el index 16af67494..1daaf8623 100644 --- a/ebdb.el +++ b/ebdb.el @@ -2056,11 +2056,11 @@ Eventually this method will go away." (defclass ebdb-field-image (ebdb-field) ((image :type (or null string symbol) - :initarg :image)) + :initarg :image + ;; "`," is used to trick EIEIO into evaluating the form. + :initform `,ebdb-image)) :human-readable "image") -(eieio-oset-default 'ebdb-field-image 'image ebdb-image) - (cl-defmethod ebdb-read ((image (subclass ebdb-field-image)) &optional slots obj) (let ((existing (when obj (slot-value obj 'image))) value) @@ -3378,7 +3378,8 @@ executable. When a symbol, assume an Elisp function." ;; I don't think I can actually set this to `ebdb-record': the ;; type needs to be a class, not an instance. Can I do that? :type symbol - :initform nil + ;; "`," is used to trick EIEIO into evaluating the form. + :initform `,ebdb-default-record-class :custom symbol :documentation "The default EIEIO class for records in this database. Must @@ -3389,11 +3390,6 @@ not be instantiated directly, subclass it instead." :allow-nil-initform t :abstract t) -;; I was told not to use this in Gnus, but I don't remember why. I -;; suspect it was backward compatibility, and that's obviously already -;; out the window. -(oset-default 'ebdb-db record-class ebdb-default-record-class) - (cl-defmethod initialize-instance ((db ebdb-db) &optional slots) "Make sure DB has a uuid." (unless (and (slot-boundp db 'uuid)