From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Vladimir Sedach Newsgroups: gmane.emacs.bugs Subject: bug#64311: [PATCH] Fix shell-dirtrack-mode showing up as enabled in unrelated buffers Date: Fri, 30 Jun 2023 10:47:05 -0600 Message-ID: <87r0ps7v92.fsf@t510.orion.oneofus.la> References: <87pm5h4iy1.fsf@t510.orion.oneofus.la> <83h6qtw3tq.fsf@gnu.org> <87wmzp2e0d.fsf@t510.orion.oneofus.la> <831qhwx5qf.fsf@gnu.org> <87zg4k76es.fsf@orphne.orion.oneofus.la> <83mt0jvmfu.fsf@gnu.org> <87r0pva66g.fsf@t510.orion.oneofus.la> <834jmrv3p6.fsf@gnu.org> <87o7kz9wej.fsf@t510.orion.oneofus.la> <83wmzmuaq7.fsf@gnu.org> <87wmzmb5fl.fsf@t510.orion.oneofus.la> <83a5wita0p.fsf@gnu.org> <87ttuqax6d.fsf@t510.orion.oneofus.la> <835y75tsn7.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25012"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: mu4e 1.4.15; emacs 28.2 Cc: 64311@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jun 30 18:48:20 2023 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 1qFHIA-0006JG-JP for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 30 Jun 2023 18:48:19 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qFHHw-0001fU-V9; Fri, 30 Jun 2023 12:48:04 -0400 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 1qFHHu-0001ei-QU for bug-gnu-emacs@gnu.org; Fri, 30 Jun 2023 12:48:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qFHHu-0001w5-EA for bug-gnu-emacs@gnu.org; Fri, 30 Jun 2023 12:48:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qFHHu-0002sL-A6 for bug-gnu-emacs@gnu.org; Fri, 30 Jun 2023 12:48:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Vladimir Sedach Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 30 Jun 2023 16:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 64311 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 64311-submit@debbugs.gnu.org id=B64311.168814363210844 (code B ref 64311); Fri, 30 Jun 2023 16:48:02 +0000 Original-Received: (at 64311) by debbugs.gnu.org; 30 Jun 2023 16:47:12 +0000 Original-Received: from localhost ([127.0.0.1]:55671 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qFHH6-0002op-AV for submit@debbugs.gnu.org; Fri, 30 Jun 2023 12:47:12 -0400 Original-Received: from robust-software.ca ([174.136.98.50]:30461) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qFHH3-0002of-66 for 64311@debbugs.gnu.org; Fri, 30 Jun 2023 12:47:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=virgo1; bh=D6B5Fmj2i8jVo RQioHINZvjuWE2ZIIjIB8VEjBCySMQ=; h=date:in-reply-to:subject:cc:to: from:references; d=oneofus.la; b=k3j7xJGc+AXL/ixUsINL3f6HAamv37fvyfttt nI/re5fJ27fce4aDUTWMDPxok6IwAFnsxRPc7h/2zHXvZhjxSwQTkaRMWGNFkpQ3sspdJ7 o61xh42ufMKBie35kHjCikxn6Th2EOSjgUV0dqkwhdEHCR/G74QaGS2Wp7/ttzsk= Original-Received: from t510.orion.oneofus.la (node-1w7jra28qzk6zk1mmpq1riwle.ipv6.telus.net [2001:56a:f913:3c01:e590:52ea:9935:f482]) by virgo1.oneofus.la (OpenSMTPD) with ESMTPSA id b78f070e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 30 Jun 2023 09:47:07 -0700 (PDT) In-reply-to: <835y75tsn7.fsf@gnu.org> 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:264324 Archived-At: Eli Zaretskii writes: > The discussion was long because I couldn't connect between the problem > and the changes you proposed. The solution I thought about > immediately was just to change the value of the variable. The rest > was getting you to agree that such a change would indeed solve the > problem (although you disagree it's the right solution). The problem is not with the variable value, the problem is with the variable binding. If you look at shell.el right now, there are 3 places where the binding changes: 1. shell.el:349: defvaralias 'shell-dirtrack-mode 'shell-dirtrackp 2. shell.el:351: defvar shell-dirtrackp t 3. shell.el:1170: define-minor-mode shell-dirtrack-mode 1. assigns the alias 2. assigns a global binding type to shell-dirtrackp 3. assigns a buffer-local binding type to shell-dirtrackp If you look at 2, you don't see that shell-dirtrackp becomes buffer-local. If you look at 3, you don't see that shell-dirtrack-mode gets a default value. Where is the bug? Is it in step 1, 2, 3, or all of the above? Notice how the change in 9c3eeba4db26ddaeead100beea7a96f9fa640918 had another unintended effect: before the change, shell-dirtrackp would affect every shell-mode buffer; now setting the variable affects only the current buffer. Whether you consider that a bug or an "accidental improvement" is irrelevant. That commit was to fix compiler warnings, not to change global behavior. So this was far from obvious for Glenn Morris, and it's not obvious to you, and you are the Emacs maintainer. How is someone who is not an elisp expert supposed to figure this out? How are people supposed to avoid more bugs when touching this variable in future shell.el changes? > Such confusion can be prevented by adding appropriate comments. Obviously not in this case, because comments do not affect how variable bindings change. > By contrast, removing the variable, or declaring it obsolete, is > backward-incompatible change in behavior, which we try to avoid at all > costs. In this case, I see absolutely no justification for such > backward incompatibility. We wouldn't be able to defend such a change > if it caused someone annoyance or, worse, breakage of their Emacs > setup and usage. If you think the patch should do a defvaralias instead of a define-obsolete-variable-alias, that's fine. The reason I preferred to mark it obsolete is that variable aliases cause subtle bugs like this, and IMO are generally a bad idea. -- Vladimir Sedach