From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Byte-compilation of custom themes Date: Sat, 12 May 2018 10:04:10 +0300 Message-ID: <831sehnymt.fsf@gnu.org> References: <87efmk2qk0.fsf@tcd.ie> <87vafjhu04.fsf@tcd.ie> <87po5po7ul.fsf@tcd.ie> <87o9hoxm0w.fsf@tcd.ie> <83fu2ynvda.fsf@gnu.org> <87zi169q9y.fsf@tcd.ie> <83a7t6nlmf.fsf@gnu.org> <877eo9kjnl.fsf@tcd.ie> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1526108572 26578 195.159.176.226 (12 May 2018 07:02:52 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 12 May 2018 07:02:52 +0000 (UTC) Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org To: "Basil L. Contovounesios" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat May 12 09:02:48 2018 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 1fHOYK-0006mX-1S for ged-emacs-devel@m.gmane.org; Sat, 12 May 2018 09:02:48 +0200 Original-Received: from localhost ([::1]:52233 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fHOaP-0005s9-Fn for ged-emacs-devel@m.gmane.org; Sat, 12 May 2018 03:04:57 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46134) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fHOZi-0005k2-IP for emacs-devel@gnu.org; Sat, 12 May 2018 03:04:15 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fHOZf-0007Cn-EK for emacs-devel@gnu.org; Sat, 12 May 2018 03:04:14 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:53615) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fHOZf-0007Ch-BZ; Sat, 12 May 2018 03:04:11 -0400 Original-Received: from [176.228.60.248] (port=1508 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fHOZe-00062j-Lf; Sat, 12 May 2018 03:04:11 -0400 In-reply-to: <877eo9kjnl.fsf@tcd.ie> (contovob@tcd.ie) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e 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:225255 Archived-At: > From: "Basil L. Contovounesios" > Cc: , > Date: Fri, 11 May 2018 21:43:42 +0100 > > > We should at least have a comment there that we are relying on > > custom-theme--load-path to do the test, and perhaps also an assertion. > > Do you mean a cl-assertion, or an emulation thereof? I meant cl-assert. > Either way, what is the benefit of barfing before directory-files does? That you can control the text of the error message, and make it more user-friendly. Also, an assertion clearly means we intended this not to happen, rather than that the code failed to handle some valid situation. Once again, the minimum I requested was a comment; it's up to you whether to use cl-assert. But see below. > Wouldn't a docstring for custom-theme--load-path and a comment in > custom-available-themes suffice for the reader (they do for me)? We've seen many situations where code guidelines are violated, for whatever reasons, so just documenting stuff is not enough. Moreover, custom-theme--load-path might some day change and invalidate our assumption. The way to prevent that from happening could be either having the assertion in the caller, or by adding a test to the test suite that makes sure the list returned by custom-theme--load-path never includes anything but existing directories. > (dolist (dir (custom-theme--load-path)) > + ;; `custom-theme--load-path' promises DIR exists. "promises DIR exists and is a directory", I think. > (defun custom-theme--load-path () > + "Expand `custom-theme-load-path' into list of directories. > +Only existing directories are included in the path returned." I'd find this text a bit of a riddle. How about this instead: "Expand `custom-theme-load-path' into list of directories. Members of `custom-theme-load-path' that either don't exist or are not directories are omitted from the expansion." Thanks.