From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#64311: [PATCH] Fix shell-dirtrack-mode showing up as enabled in unrelated buffers Date: Sun, 02 Jul 2023 09:39:28 +0300 Message-ID: <831qhqq0kf.fsf@gnu.org> 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> <87r0ps7v92.fsf@t510.orion.oneofus.la> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="10229"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 64311@debbugs.gnu.org To: Vladimir Sedach Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Jul 02 08:40:24 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 1qFqky-0002WB-LJ for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 02 Jul 2023 08:40:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qFqke-00080q-QS; Sun, 02 Jul 2023 02:40: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 1qFqkc-00080c-Tc for bug-gnu-emacs@gnu.org; Sun, 02 Jul 2023 02:40:03 -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 1qFqkc-0000nk-KT for bug-gnu-emacs@gnu.org; Sun, 02 Jul 2023 02:40:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qFqkc-0006qg-F7 for bug-gnu-emacs@gnu.org; Sun, 02 Jul 2023 02:40:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 02 Jul 2023 06:40: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.168827994426241 (code B ref 64311); Sun, 02 Jul 2023 06:40:02 +0000 Original-Received: (at 64311) by debbugs.gnu.org; 2 Jul 2023 06:39:04 +0000 Original-Received: from localhost ([127.0.0.1]:59068 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qFqjf-0006p8-Uw for submit@debbugs.gnu.org; Sun, 02 Jul 2023 02:39:04 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:48240) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qFqjc-0006od-P8 for 64311@debbugs.gnu.org; Sun, 02 Jul 2023 02:39:02 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qFqjX-0000is-Ad; Sun, 02 Jul 2023 02:38:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=kq3K4tQ6ePzLwG9sJ1DUWYgy6cxxLqumZeTP2CIv6dQ=; b=ZOQ0sgxjtuep WHH2/bmRL6O6z8jDTr/NaGULxLZV7eB+JYTub0NW9+k2ArB4Km7/hHOlUyJj3fAZl1LFqXKfLiVEP jFWZds18e5yYtubfO1k01YMdItOlN1ApsoY51F4u23O46GqfsCAR9lnnUH50UtucCqdohExwOgLs0 BNUCd4DGxPQRUw7oDvZxrS9JlE+htdSQMhQVjcg9uq4l0omM/jNaDKpDV2dK8XyoDCZVqE7/5ih6I jCkxrC1K4E+dEaxDbC1FtMlHK22LlntSFCoMBF2JHUgagtPJXYsX3jkpjffpbaPWlAKc/OHRkx56G oqP1D/HBvNmGijJg4ARixA==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qFqjW-0005Yx-Od; Sun, 02 Jul 2023 02:38:55 -0400 In-Reply-To: <87r0ps7v92.fsf@t510.orion.oneofus.la> (message from Vladimir Sedach on Fri, 30 Jun 2023 10:47:05 -0600) 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:264449 Archived-At: > From: Vladimir Sedach > Cc: 64311@debbugs.gnu.org > Date: Fri, 30 Jun 2023 10:47:05 -0600 > > 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. I think the alternative patch below solves all those issues. If you disagree, pleased tell what left-over problems you see after applying that patch. > 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. I think this change is for the better, and the 5 years since that change seem to at least tell us it had no negative effects on users. And your proposed patch AFAICT does nothing to change this aspect of that old commit, does it? > > 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. shell.el already uses defvaralias. > The reason I preferred to mark it obsolete is that variable aliases > cause subtle bugs like this, and IMO are generally a bad idea. It's too late for such considerations, since this alias has been with us for many years, and it is quite possible some users depend on it in their setups. So changes that don't break their setups are preferable. Here's the patch I propose: diff --git a/lisp/shell.el b/lisp/shell.el index 5cf108b..4bbd295 100644 --- a/lisp/shell.el +++ b/lisp/shell.el @@ -348,8 +348,10 @@ shell-dirstack (defvaralias 'shell-dirtrack-mode 'shell-dirtrackp) -(defvar shell-dirtrackp t - "Non-nil in a shell buffer means directory tracking is enabled.") +(defvar-local shell-dirtrackp nil + "Non-nil in a shell buffer means directory tracking is enabled. +Directory tracking (`shell-dirtrack-mode') is automatically enabled +when `shell-mode' is activated.") (defvar shell-last-dir nil "Keep track of last directory for ksh `cd -' command.") @@ -1129,6 +1131,7 @@ shell-extract-num (define-minor-mode shell-dirtrack-mode "Toggle directory tracking in this shell buffer (Shell Dirtrack mode). +This assigns a buffer-local non-nil value to `shell-dirtrackp'. The `dirtrack' package provides an alternative implementation of this feature; see the function `dirtrack-mode'. Also see