From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded. Date: Sun, 10 Dec 2023 00:10:53 -0500 Message-ID: References: <170177277759.6083.12155719482709043212@vcs2.savannah.gnu.org> <20231205103937.E1D65C405A8@vcs2.savannah.gnu.org> <020ab182-0e3d-4e8d-9415-c93863b95638@vodafonemail.de> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="11026"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: emacs-devel@gnu.org, Eli Zaretskii To: Jens Schmidt Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Dec 10 06:12:03 2023 Return-path: Envelope-to: ged-emacs-devel@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 1rCC6k-0002ZS-Kj for ged-emacs-devel@m.gmane-mx.org; Sun, 10 Dec 2023 06:12:02 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rCC5s-0006dq-86; Sun, 10 Dec 2023 00:11:08 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rCC5q-0006di-Bd for emacs-devel@gnu.org; Sun, 10 Dec 2023 00:11:06 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rCC5n-0003zN-89; Sun, 10 Dec 2023 00:11:05 -0500 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 3C9AF8001F; Sun, 10 Dec 2023 00:11:01 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1702185059; bh=rYOYTyqMkgSPc02FSNm3N4a52AceAGm1+mObNzw48s8=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=bVg9eDb0h/YHpRcgMg8OZ+Q2AcJWCCN5Ve0jce+4mnB9V49jFEdktEK4srJj7x5mq J/Or8XqoJiDTG/kjFzYqGIY0n4VR2xTUjf5zjsXzVzr/CIYsz7am4meJvX9wDtB7KD p83+henFR5YuvkSFAgNJOJuX/N3CYTXeVcT9pYBCfGIfABNgARdhtyloR6BkP+QBcv dYpittEzZR7jvl6BZa3TtWzTwkFl5AJ0dU77nAS8+ea29oyl51tsLpEd4X+i0EYBSS PRS6lzUoua9M/TcHynEwbXmS4wci8EskeQva0mE1U75pY5gZyzAcSN0y+VU1upAoZt cZ7nIXj2XVIHA== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id CA2A680312; Sun, 10 Dec 2023 00:10:59 -0500 (EST) Original-Received: from pastel (unknown [45.72.207.126]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 9E14F12040D; Sun, 10 Dec 2023 00:10:59 -0500 (EST) In-Reply-To: (Jens Schmidt's message of "Thu, 7 Dec 2023 23:25:44 +0100") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 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-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:313650 Archived-At: I pushed a cleaner version of the code, split into more understandable individual patches into the branch `scratch/no-ls-lisp-advice`. > - The docstring of `ls-lisp--insert-directory' still makes references > to its previous advice nature, this probably should be ironed out. Fixed. > - The docstring of `insert-directory-program' does not very explicitly > mention that its value could be nil. (Nothing introduced by your > patch, just noticed it.) Still left as is. Maybe we should document more prominently that nil is a valid value now, but Eli seemed to prefer not to go there. > - And the custom spec of `insert-directory-program' isn't prepared for > nil as well, but that is probably OK since it should be used for a > generated default value only? If we do want to support nil more actively we should definitely include it in the `:type`. > - AFAICT you call `files--insert-directory-program' as a predicate > only. Probably we should rename it to `...-p' or > `files--use-insert-directory-program'? Fixed. > - And here is a feeble attempt to provide a docstring: > > "Return wether `insert-directory' should use an external program. > Take into consideration `ls-lisp-use-insert-directory-program' if that is > available." Done (well, I ended up changing it afterwards). > - Not sure whether that makes a difference, but: Without your patch the > advice was conditional on whether ls-lisp was loaded, now the usage of > `ls-lisp--insert-directory' is conditional on whether > `ls-lisp-use-insert-directory-program' is bound. What if a user has > a .emacs with a customization or `setq' of `ls-l-u-i-d-p' to nil, which > she uses both on Windows and Unix? As mentioned, I don't think this is really a new bug. It may appear more often, but I don't think there's much we can do about it, and I'd argue that the new behavior is better because it doesn't magically change when some Lisp file is loaded. > - Your fix in `file-expand-wildcards' > > (if (equal "" nondir) > (push (or dir nondir) contents) > > > is hard to digest at 11PM, so a comment won't hurt here, I guess. Added. > Plus it seems to violate the order promise done in the docstring of > the function ("... sorted in the `string<' order"). Fixed. > - Now the logic gets harder and harder, so I may be wrong here. The > docstring of `dired-insert-directory' seems to handle FILE-LIST > exclusive to WILDCARD, as if WILDCARDS do not get expanded if > FILE-LIST is non-nil: > > If FILE-LIST is non-nil, list only those files. > Otherwise, if WILDCARD is non-nil, expand wildcards; > > but your patch changes that, no? This is now factored into its own commit (included in the branch): commit f7cf85c3879c6857e8478bef41cce25a94759fb8 Author: Stefan Monnier Date: Sat Dec 9 22:55:32 2023 -0500 (dired-insert-directory): Obey `file-list` and `wildcard` Commit 6f6639d6ed6c's support for wildcards in directories failed to obey `file-list` and `wildcard` arguments. Fix it. * lisp/dired.el (dired-insert-directory): Expand directory wildcards only if `file-list` is nil and `wildcard` is non-nil. Also, refer back to `dir-wildcard` instead of recomputing it. (dired-readin-insert): Pass a non-nil `wildcard` when wildcard expansion might be needed to preserve former behavior. The branch currently doesn't include any doc nor etc/NEWS changes. I'm not sure what to say there, because this is an internal change. Let me know if you think something's needed in this respect, and if so what. Stefan