From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "J.P." Newsgroups: gmane.emacs.bugs Subject: bug#56340: 29.0.50; [PATCH] Don't ask people to order requires. Date: Mon, 04 Jul 2022 16:36:08 -0700 Message-ID: <87czekcuxj.fsf__16804.1165381268$1656977839$gmane$org@neverwas.me> References: <87fsjk6eml.fsf@dick> <87o7y71yr1.fsf@gnus.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7996"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: Lars Ingebrigtsen , 56340@debbugs.gnu.org, Amin Bandali , emacs-erc@gnu.org To: dick.r.chiang@gmail.com Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Jul 05 01:37:13 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o8Vct-0001ud-Uz for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 05 Jul 2022 01:37:12 +0200 Original-Received: from localhost ([::1]:59842 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o8Vcs-0001cN-7r for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 04 Jul 2022 19:37:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46342) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o8Vck-0001cE-3W for bug-gnu-emacs@gnu.org; Mon, 04 Jul 2022 19:37:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54924) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o8Vcj-0005Ak-Qd for bug-gnu-emacs@gnu.org; Mon, 04 Jul 2022 19:37:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o8Vcj-0005bG-Ke for bug-gnu-emacs@gnu.org; Mon, 04 Jul 2022 19:37:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "J.P." Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 04 Jul 2022 23:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 56340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 56340-submit@debbugs.gnu.org id=B56340.165697778321476 (code B ref 56340); Mon, 04 Jul 2022 23:37:01 +0000 Original-Received: (at 56340) by debbugs.gnu.org; 4 Jul 2022 23:36:23 +0000 Original-Received: from localhost ([127.0.0.1]:48821 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o8Vc6-0005aJ-N8 for submit@debbugs.gnu.org; Mon, 04 Jul 2022 19:36:23 -0400 Original-Received: from mail-108-mta132.mxroute.com ([136.175.108.132]:41541) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o8Vc3-0005a2-GH for 56340@debbugs.gnu.org; Mon, 04 Jul 2022 19:36:20 -0400 Original-Received: from filter006.mxroute.com ([140.82.40.27] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta132.mxroute.com (ZoneMTA) with ESMTPSA id 181cb92512c000a238.001 for <56340@debbugs.gnu.org> (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Mon, 04 Jul 2022 23:36:12 +0000 X-Zone-Loop: 345d2cb86f76a90917fdb4b0e7f73f7e3ca52d5f09a3 X-Originating-IP: [140.82.40.27] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID: In-Reply-To:Date:References:Subject:Cc:To:From:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=zIj5Za1zU7wV71cHzlUsHw/yRjne4UrWClo7HN2dttQ=; b=LhAT7abtXUEDykk6ZWEJnRvH90 DaEMd5YZTLuUZZrbFJ81DOWuzjNiv5Qwj904aZWEkzEGINFBFfLtb0/QJP6YaYH3RBS3A5QWyWXFz kyVwJb5Zt3xV4s/Xnbv0wwgLAgxArKf7Io07684EBCuRUIs5tsml3VKeYtV8OLaMkcPes97QIEeMT /9+Hds8FbCy07bD0CKHs0wG4yKFlgpZhlyy6v1K5mrRmxJoVfZjSkSNGBRqfVgb1HS/6ZcjrW1pjh eSkO1RlCkjuRKVbgXwtgKaezZ2OhtoB+cdYVSKJX8zUmi2MxSPLj83+GxgAGLbCgn4BaR7PNF7XS5 EmgJCtnA==; In-Reply-To: <87o7y71yr1.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sat, 02 Jul 2022 14:32:18 +0200") X-AuthUser: masked@neverwas.me X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:236074 Archived-At: Hi Dick, Lars, people, Lars Ingebrigtsen writes: > dick.r.chiang@gmail.com writes: > >> Asking people to order require's is about as effective >> as asking kids to keep off the grass. > > It looks like this patch is rearranging some dependencies to make erc > load in a more logical way? > > I haven't tried the patch, but skimming it, it seems to make some sense. > I've added the erc maintainers to the CCs. This bug seems to be in answer to #54825, which, Dick, I guess you found wanting enough to serve as your next meal (and its author simple enough to adopt as your next mark). Perhaps all is right with the world. Regardless of whatever ancillary motivations might have prompted that perceived half measure from me [1], given the direction I'd like to see ERC move in, a little turbulence in this arena is likely inevitable. So I suppose now's a good a time as any to rip this Band Aid clean off. And having myself on occasion gamed out similar changes to these, I can somewhat appreciate what appears to be an earnest attempt at leaving a relatively light footprint overall. > * lisp/erc/erc-backend.el (erc-networks--id, erc-reuse-buffers, > erc-kill-server-buffer-on-quit, erc-insert-marker, erc-input-marker, > erc-hide-prompt, erc-default-recipients, erc-prompt-hidden, Is the convention here really to list all of these? Or is this like all part of the show (or maybe both)? (Sorry, I, Philistine.) If yes to the first, I guess I've been missing out but will fall in line ASAP. FWIW, grepping the change logs for "forward decl" only returns a handful of hits. > @@ -1673,12 +1880,15 @@ erc--parse-isupport-value > (split-string value ",") > (list value))))) > > -;; FIXME move to erc-compat (once we decide how to load it) > -(defalias 'erc--with-memoization > - (cond > - ((fboundp 'with-memoization) #'with-memoization) ; 29.1 > - ((fboundp 'cl--generic-with-memoization) #'cl--generic-with-memoizati= on) > - (t (lambda (_ v) v)))) Hmm, yeah, thanks. Caught that one on the chin on the way out the door and was hoping no one would notice until I could sweep it under the rug all furtive like. (It's definitely one for the ages, though, I'll give you that: hall of shame material for sure.) I'm kinda surprised the compiled version runs at all instead of signaling an "invalid function" or something. Either way, short of our keeping it as a pet and renaming it `with-bath-salts' (or `with-kung-flu' or whatever), I doubt it'll be long for this world in any form once we get our compat ducks in a row. > +(declare-function erc-nickname-in-use "erc") > (define-erc-response-handler (433) > - "Login-time \"nick in use\"." nil > - (erc-nickname-in-use (cadr (erc-response.command-args parsed)) > - "already in use")) > + "Login-time \"nick in use\"." nil > + (erc-nickname-in-use (cadr (erc-response.co= mmand-args parsed)) > + "already in use")) Indentation thing aside, I wasn't aware we were allowed to forgo including signatures in these `declare-function' forms. But if that's the deal, I guess it makes for a cleaner, less cluttered look overall. >From "Rework mutual dependency yada yada" (bug#54825) [2]: > ;;;; Variables and options > modified lisp/erc/erc.el > @@ -130,7 +130,26 @@ erc-scripts > "Running scripts at startup and with /LOAD." > :group 'erc) > > -(require 'erc-backend) > +;; Defined in erc-backend > +(defvar erc--server-reconnecting) > +(defvar erc-channel-members-changed-hook) > +(defvar erc-server-367-functions) > +(defvar erc-server-announced-name) > [...] > > @@ -48,6 +48,27 @@ erc--read-time-period > (cl-letf (((symbol-function 'read-string) (lambda (&rest _) "1d"))) > (should (equal (erc--read-time-period "foo: ") 86400)))) > > +(ert-deftest erc--meta--backend-dependencies () > + (with-temp-buffer > + (insert-file-contents-literally Your changes obsolete most of these `defvars' and the associated test. Not saying it should fall on you to tidy up, but "someone" should remove them. Which gets me wondering (not for the first time): all these `defvar's and `declare-function's seem a bit litter-prone, no? Not sure if any diagnostic tooling already catches extraneous ones orphaned by refactoring, but I for one could use such a feature. > Don't do this. > > Asking people to order require's is about as effective > as asking kids to keep off the grass. How about a more pedestrian commit subject, like maybe one mentioning ERC or something it defines? (But just rolling the dice on whatever malaprop-laden ESL lesson I cook up also works.) > @@ -196,7 +209,8 @@ erc-send-distinguish-noncommands > (not (string-match "\n.+$" string)) > (memq cmd-fun erc-noncommands-list)) > ;; Inhibit sending this string. > - (setf (erc-input-insertp state) nil)))) > + (with-no-warnings ;how to declare-function a cl-defmethod? > + (setf (erc-input-insertp state) nil))))) > > ;;; IRC control character processing. > (defgroup erc-control-characters nil When I byte compile erc-goodies.el and then, with emacs -Q, do (require 'erc) (let ((input (make-erc-input :string "/me nods" :sendp t :insertp t))) (erc-send-distinguish-noncommands input) input) I get Symbol=E2=80=99s function definition is void: \(setf\ erc-input-insertp\) Adding this somewhere atop goodies (eval-when-compile (provide 'erc-goodies) (require 'erc)) seems to give the expected output: =3D> #s(erc-input "/me nods" nil t) Although it also introduces a couple new compiler warnings as well, so what do I know? (Don't answer that.) This or similar (in emacs -Q) may offer some insights: (trace-function-foreground 'gv-get) (byte-compile-file "lisp/erc/erc-goodies.el") Basically, I don't really care what happens with `erc-goodies' in the extreme short term (barring some immediate ELPA release) as long as we preserve as much behavior and blame as is reasonable. Hopefully, requiring goodies will become a thing of the past after this [3] or something superior lands, meaning expect new bugs from me shortly. (That's your cue to flee, people.) Thanks, J.P. =20=20=20=20 [1] Not that anyone should care, but there was, for example, the matter of damage control for some unfortunate advice that found its way to a couple "exemplar" configs, which ended up hooking all their setup on `erc-backend' as a sad workaround for these ancient dependency problems. So, some of my aim might have been diverted toward not breaking those until we had a worthy feature to trade for the churn. Add to that a general reticence to disturb large swaths of ancient blame and we get closer to my (perhaps failed) attempt at reining in only the most unwieldy parts of the dependency cycle rather than doing the bold thing (as is being proposed here). [2] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3D0f52e7ac [3] https://ttm.sh/es3