From: Vladimir Sedach <vas@oneofus.la>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 64311@debbugs.gnu.org
Subject: bug#64311: [PATCH] Fix shell-dirtrack-mode showing up as enabled in unrelated buffers
Date: Tue, 27 Jun 2023 18:07:27 -0600 [thread overview]
Message-ID: <87zg4k76es.fsf@orphne.orion.oneofus.la> (raw)
In-Reply-To: <831qhwx5qf.fsf@gnu.org>
Eli Zaretskii <eliz@gnu.org> writes:
> Why is that a problem. I understand when it causes irrelevant minor
> mode to be shown by "C-h m", but why should anyone care that some
> global variable is non-nil?
My understanding is that (define-minor-mode X-mode ...) defines a
variable X-mode (the docstring for define-minor-mode calls this a
"control variable") that is supposed to be t when the mode is
enabled, and nil when the mode is not enabled.
Right now the variable shell-dirtrack-mode has a value of t, even
when the mode is not enabled.
> In any case, I don't think a fix (if we need one) should be so
> complicated. Why do we need all those changes, including making the
> variable obsolete and moving the mode from its place in shell.el to
> another place there? If all you want is to make this variable
> buffer-local, just making it buffer-local is all that's needed,
> right?
shell-dirtrack-mode is already made buffer-local by
define-minor-mode.
The problem is shell-dirtrackp and its default value.
What is shell-dirtrackp?
Looking at VC-history for shell-dirtrackp, there are 2 commits:
--8<---------------cut here---------------start------------->8---
commit 9c3eeba4db26ddaeead100beea7a96f9fa640918
Author: Glenn Morris <rgm@gnu.org>
Date: Fri Apr 20 18:34:39 2018 -0400
The tedious game of whack-a-mole with compiler warnings continues
...
diff --git a/lisp/shell.el b/lisp/shell.el
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -317,4 +317,6 @@
+(defvaralias 'shell-dirtrack-mode 'shell-dirtrackp)
+
(defvar shell-dirtrackp t
"Non-nil in a shell buffer means directory tracking is enabled.")
commit b493a9b2af805a3097fe53fd472884c268248146
Author: Richard M. Stallman <rms@gnu.org>
Date: Wed Mar 2 16:55:16 1994 +0000
(shell-dirtrackp): Variable definition added.
diff --git a/lisp/shell.el b/lisp/shell.el
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -226,1 +223,4 @@
+(defvar shell-dirtrackp t
+ "Non-nil in a shell buffer means directory tracking is enabled.")
+
--8<---------------cut here---------------end--------------->8---
So it looks like in 1994 rms introduced the variable shell-dirtrackp,
before define-minor-mode and the X-mode variable convention. Judging
by the documentation string, shell-dirtrackp was intended to do what
the automatically defined X-mode variables do now.
Then in 2018 rgm aliased shell-dirtrackp to shell-dirtrack-mode to
fix a compiler warning. This introduced an incorrect default value
for shell-dirtrack-mode.
The variable shell-dirtrackp should also have been marked obsolete in
rgm's commit.
I moved the definition of shell-dirtrack-mode above the first use of
the variable shell-dirtrack-mode so there would be no compiler
warning (this is noted in the commit message). This also puts the
definition of shell-dirtrack-mode right after the long comment for
the Directory tracking section explaining the mode's purpose, a nice
unintended benefit.
> But first, let's talk about the problem: why is shell-dirtrack-mode
> being t a problem?
If you have a hook that tests if shell-dirtrack-mode is turned on by
looking at the value of the variable shell-dirtrack-mode, that hook
will not work correctly.
--
Vladimir Sedach
next prev parent reply other threads:[~2023-06-28 0:07 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-27 4:39 bug#64311: [PATCH] Fix shell-dirtrack-mode showing up as enabled in unrelated buffers Vladimir Sedach
2023-06-27 11:18 ` Eli Zaretskii
2023-06-27 14:09 ` Vladimir Sedach
2023-06-27 15:52 ` Eli Zaretskii
2023-06-28 0:07 ` Vladimir Sedach [this message]
2023-06-28 11:46 ` Eli Zaretskii
2023-06-28 16:43 ` Vladimir Sedach
2023-06-28 18:31 ` Eli Zaretskii
2023-06-28 20:14 ` Vladimir Sedach
2023-06-29 4:57 ` Eli Zaretskii
2023-06-29 16:26 ` Vladimir Sedach
2023-06-29 18:10 ` Eli Zaretskii
2023-06-29 19:24 ` Vladimir Sedach
2023-06-30 5:40 ` Eli Zaretskii
2023-06-30 16:47 ` Vladimir Sedach
2023-07-02 6:39 ` Eli Zaretskii
2023-07-03 17:03 ` Vladimir Sedach
2023-07-03 17:17 ` Eli Zaretskii
2023-07-04 14:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-07-04 16:05 ` Eli Zaretskii
2023-07-04 18:34 ` Vladimir Sedach
2023-07-04 20:36 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-07-04 22:27 ` Vladimir Sedach
2023-07-04 23:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-07-06 20:30 ` Vladimir Sedach
2023-07-08 8:30 ` Eli Zaretskii
2023-07-08 16:18 ` Vladimir Sedach
2023-07-08 16:31 ` Eli Zaretskii
2023-07-04 3:32 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-07-04 11:21 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zg4k76es.fsf@orphne.orion.oneofus.la \
--to=vas@oneofus.la \
--cc=64311@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).