* bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly
@ 2017-03-02 13:48 Reuben Thomas
2017-03-24 0:06 ` npostavs
0 siblings, 1 reply; 5+ messages in thread
From: Reuben Thomas @ 2017-03-02 13:48 UTC (permalink / raw)
To: 25936
[-- Attachment #1.1: Type: text/plain, Size: 728 bytes --]
I noticed that whitespace-mode was not correctly diagnosing whitespace
problems in some files.
On investigation, this was because whitespace-report-region didn't call
whitespace-ensure-local-variables, so it didn't catch changes to
indent-tabs-mode in file-find hook functions.
However, on further investigation, I can't see why these local variables
are needed. Currently, whitespace-mode makes buffer-local copies of
indent-tabs-mode and tab-width, with names prefixed by "whitespace-". But
these variables are buffer-local if necessary already, and whitespace-mode
never tries to change them.
Hence, I attach a patch for review which simply removes these (apparently
unnecessary) local variables.
--
http://rrt.sc3d.org
[-- Attachment #1.2: Type: text/html, Size: 1313 bytes --]
[-- Attachment #2: 0001-Fix-reading-of-tab-settings-in-whitespace-mode.patch --]
[-- Type: text/x-patch, Size: 6204 bytes --]
From 1ca64fa2f7c44112f27dc56139d7319d3e95cca5 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Thu, 2 Mar 2017 12:50:06 +0000
Subject: [PATCH] Fix reading of tab settings in whitespace-mode
lisp/whitespace.el (whitespace-indent-tabs-mode)
whitespace-tab-width): Remove these variables. The underlying
variables `indent-tabs-mode' and `tab-width' are already buffer-local
when needed, and whitespace-mode never changes them.
(whitespace-ensure-local-variables): Remove this function, which only
existed to set the above variables.
(whitespace-cleanup-region, whitespace-regexp)
(whitespace-indentation-regexp, whitespace-report-region)
(whitespace-turn-on, whitespace-color-on): Adjust these functions to
use `indent-tabs-mode' and `tab-width' directly, and not call
`whitespace-ensure-local-variables'.
---
lisp/whitespace.el | 44 ++++++++++++++------------------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index 6c4f59d..c44e2be 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1134,12 +1134,6 @@ whitespace-toggle-option-alist
(defvar whitespace-active-style nil
"Used to save locally `whitespace-style' value.")
-(defvar whitespace-indent-tabs-mode indent-tabs-mode
- "Used to save locally `indent-tabs-mode' value.")
-
-(defvar whitespace-tab-width tab-width
- "Used to save locally `tab-width' value.")
-
(defvar whitespace-point (point)
"Used to save locally current point value.
Used by function `whitespace-trailing-regexp' (which see).")
@@ -1415,12 +1409,6 @@ whitespace-cleanup
;; PROBLEM 6: `tab-width' or more SPACEs after TAB
(whitespace-cleanup-region (point-min) (point-max)))))
-(defun whitespace-ensure-local-variables ()
- "Set `whitespace-indent-tabs-mode' and `whitespace-tab-width' locally."
- (set (make-local-variable 'whitespace-indent-tabs-mode)
- indent-tabs-mode)
- (set (make-local-variable 'whitespace-tab-width)
- tab-width))
;;;###autoload
(defun whitespace-cleanup-region (start end)
@@ -1467,11 +1455,8 @@ whitespace-cleanup-region
;; read-only buffer
(whitespace-warn-read-only "cleanup region")
;; non-read-only buffer
- (whitespace-ensure-local-variables)
(let ((rstart (min start end))
(rend (copy-marker (max start end)))
- (indent-tabs-mode whitespace-indent-tabs-mode)
- (tab-width whitespace-tab-width)
overwrite-mode ; enforce no overwrite
tmp)
(save-excursion
@@ -1512,7 +1497,7 @@ whitespace-cleanup-region
;; by SPACEs.
((memq 'space-after-tab whitespace-style)
(whitespace-replace-action
- (if whitespace-indent-tabs-mode 'tabify 'untabify)
+ (if indent-tabs-mode 'tabify 'untabify)
rstart rend (whitespace-space-after-tab-regexp) 1))
;; ACTION: replace `tab-width' or more SPACEs by TABs.
((memq 'space-after-tab::tab whitespace-style)
@@ -1531,9 +1516,9 @@ whitespace-cleanup-region
;; by SPACEs.
((memq 'space-before-tab whitespace-style)
(whitespace-replace-action
- (if whitespace-indent-tabs-mode 'tabify 'untabify)
+ (if indent-tabs-mode 'tabify 'untabify)
rstart rend whitespace-space-before-tab-regexp
- (if whitespace-indent-tabs-mode 0 2)))
+ (if indent-tabs-mode 0 2)))
;; ACTION: replace SPACEs before TAB by TABs.
((memq 'space-before-tab::tab whitespace-style)
(whitespace-replace-action
@@ -1564,25 +1549,25 @@ whitespace-replace-action
(defun whitespace-regexp (regexp &optional kind)
- "Return REGEXP depending on `whitespace-indent-tabs-mode'."
+ "Return REGEXP depending on `indent-tabs-mode'."
(format
(cond
((or (eq kind 'tab)
- whitespace-indent-tabs-mode)
+ indent-tabs-mode)
(car regexp))
((or (eq kind 'space)
- (not whitespace-indent-tabs-mode))
+ (not indent-tabs-mode))
(cdr regexp)))
- whitespace-tab-width))
+ tab-width))
(defun whitespace-indentation-regexp (&optional kind)
- "Return the indentation regexp depending on `whitespace-indent-tabs-mode'."
+ "Return the indentation regexp depending on `indent-tabs-mode'."
(whitespace-regexp whitespace-indentation-regexp kind))
(defun whitespace-space-after-tab-regexp (&optional kind)
- "Return the space-after-tab regexp depending on `whitespace-indent-tabs-mode'."
+ "Return the space-after-tab regexp depending on `indent-tabs-mode'."
(whitespace-regexp whitespace-space-after-tab-regexp kind))
@@ -1744,10 +1729,10 @@ whitespace-report-region
whitespace-report-list)))
(when (pcase report-if-bogus (`nil t) (`never nil) (_ has-bogus))
(whitespace-kill-buffer whitespace-report-buffer-name)
- ;; `whitespace-indent-tabs-mode' is local to current buffer
- ;; `whitespace-tab-width' is local to current buffer
- (let ((ws-indent-tabs-mode whitespace-indent-tabs-mode)
- (ws-tab-width whitespace-tab-width))
+ ;; `indent-tabs-mode' may be local to current buffer
+ ;; `tab-width' may be local to current buffer
+ (let ((ws-indent-tabs-mode indent-tabs-mode)
+ (ws-tab-width tab-width))
(with-current-buffer (get-buffer-create
whitespace-report-buffer-name)
(erase-buffer)
@@ -2027,7 +2012,6 @@ whitespace-turn-on
(if (listp whitespace-style)
whitespace-style
(list whitespace-style)))
- (whitespace-ensure-local-variables)
;; turn on whitespace
(when whitespace-active-style
(whitespace-color-on)
@@ -2123,7 +2107,7 @@ whitespace-color-on
,(cond
((memq 'space-before-tab whitespace-active-style)
;; Show SPACEs before TAB (indent-tabs-mode).
- (if whitespace-indent-tabs-mode 1 2))
+ (if indent-tabs-mode 1 2))
((memq 'space-before-tab::tab whitespace-active-style)
1)
((memq 'space-before-tab::space whitespace-active-style)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly
2017-03-02 13:48 bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly Reuben Thomas
@ 2017-03-24 0:06 ` npostavs
2017-04-18 12:06 ` npostavs
0 siblings, 1 reply; 5+ messages in thread
From: npostavs @ 2017-03-24 0:06 UTC (permalink / raw)
To: Reuben Thomas; +Cc: 25936, Stephen Deasey, Vinicius Jose Latorre
Reuben Thomas <rrt@sc3d.org> writes:
> However, on further investigation, I can't see why these local variables
> are needed. Currently, whitespace-mode makes buffer-local copies of
> indent-tabs-mode and tab-width, with names prefixed by "whitespace-". But
> these variables are buffer-local if necessary already, and whitespace-mode
> never tries to change them.
These variables seem to have been introduced in [1: 55d1cfe870]. I
agree that it looks like there is no need for them, but I'll wait
another couple weeks in case someone else has some insights. Adding
author of that commit (and the person mentioned as "suggesting" it) on
CC in case they might remember anything about it.
1: 2008-04-16 03:41:17 +0000 55d1cfe8703c5829bacc8d129277f1f9b33950f6
Honor the indent-tabs-mode and tab-width setting from user.
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly
2017-03-24 0:06 ` npostavs
@ 2017-04-18 12:06 ` npostavs
2017-04-19 23:25 ` Reuben Thomas
0 siblings, 1 reply; 5+ messages in thread
From: npostavs @ 2017-04-18 12:06 UTC (permalink / raw)
To: Reuben Thomas; +Cc: 25936, Stephen Deasey
npostavs@users.sourceforge.net writes:
> Reuben Thomas <rrt@sc3d.org> writes:
>
>> However, on further investigation, I can't see why these local variables
>> are needed. Currently, whitespace-mode makes buffer-local copies of
>> indent-tabs-mode and tab-width, with names prefixed by "whitespace-". But
>> these variables are buffer-local if necessary already, and whitespace-mode
>> never tries to change them.
>
> These variables seem to have been introduced in [1: 55d1cfe870]. I
> agree that it looks like there is no need for them, but I'll wait
> another couple weeks in case someone else has some insights. Adding
> author of that commit (and the person mentioned as "suggesting" it) on
> CC in case they might remember anything about it.
>
> 1: 2008-04-16 03:41:17 +0000 55d1cfe8703c5829bacc8d129277f1f9b33950f6
> Honor the indent-tabs-mode and tab-width setting from user.
I guess they won't answer. The compiler is telling me you missed a
reference of whitespace-tab-width though.
whitespace.el:2095:45:Warning: reference to free variable
‘whitespace-tab-width’
^ permalink raw reply [flat|nested] 5+ messages in thread* bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly
2017-04-18 12:06 ` npostavs
@ 2017-04-19 23:25 ` Reuben Thomas
2017-04-21 3:17 ` npostavs
0 siblings, 1 reply; 5+ messages in thread
From: Reuben Thomas @ 2017-04-19 23:25 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 25936, Stephen Deasey
[-- Attachment #1: Type: text/plain, Size: 410 bytes --]
On 18 April 2017 at 13:06, <npostavs@users.sourceforge.net> wrote:
> npostavs@users.sourceforge.net writes:
>
> I guess they won't answer. The compiler is telling me you missed a
> reference of whitespace-tab-width though.
>
> whitespace.el:2095:45:Warning: reference to free variable
> ‘whitespace-tab-width’
Thanks very much for this. Revised patch attached.
--
http://rrt.sc3d.org
[-- Attachment #2: 0006-Fix-reading-of-tab-settings-in-whitespace-mode.patch --]
[-- Type: text/x-patch, Size: 6941 bytes --]
From 9529e6499aa480342babba7524b98255610066ee Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Thu, 2 Mar 2017 12:50:06 +0000
Subject: [PATCH 6/6] Fix reading of tab settings in whitespace-mode
lisp/whitespace.el (whitespace-indent-tabs-mode)
whitespace-tab-width): Remove these variables. The underlying
variables `indent-tabs-mode' and `tab-width' are already buffer-local
when needed, and whitespace-mode never changes them.
(whitespace-ensure-local-variables): Remove this function, which only
existed to set the above variables.
(whitespace-cleanup-region, whitespace-regexp)
(whitespace-indentation-regexp, whitespace-report-region)
(whitespace-turn-on, whitespace-color-on): Adjust these functions to
use `indent-tabs-mode' and `tab-width' directly, and not call
`whitespace-ensure-local-variables'.
---
lisp/whitespace.el | 52 ++++++++++++++++++----------------------------------
1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index 6c4f59d..6aca47c 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1134,12 +1134,6 @@ whitespace-toggle-option-alist
(defvar whitespace-active-style nil
"Used to save locally `whitespace-style' value.")
-(defvar whitespace-indent-tabs-mode indent-tabs-mode
- "Used to save locally `indent-tabs-mode' value.")
-
-(defvar whitespace-tab-width tab-width
- "Used to save locally `tab-width' value.")
-
(defvar whitespace-point (point)
"Used to save locally current point value.
Used by function `whitespace-trailing-regexp' (which see).")
@@ -1415,12 +1409,6 @@ whitespace-cleanup
;; PROBLEM 6: `tab-width' or more SPACEs after TAB
(whitespace-cleanup-region (point-min) (point-max)))))
-(defun whitespace-ensure-local-variables ()
- "Set `whitespace-indent-tabs-mode' and `whitespace-tab-width' locally."
- (set (make-local-variable 'whitespace-indent-tabs-mode)
- indent-tabs-mode)
- (set (make-local-variable 'whitespace-tab-width)
- tab-width))
;;;###autoload
(defun whitespace-cleanup-region (start end)
@@ -1467,11 +1455,8 @@ whitespace-cleanup-region
;; read-only buffer
(whitespace-warn-read-only "cleanup region")
;; non-read-only buffer
- (whitespace-ensure-local-variables)
(let ((rstart (min start end))
(rend (copy-marker (max start end)))
- (indent-tabs-mode whitespace-indent-tabs-mode)
- (tab-width whitespace-tab-width)
overwrite-mode ; enforce no overwrite
tmp)
(save-excursion
@@ -1512,7 +1497,7 @@ whitespace-cleanup-region
;; by SPACEs.
((memq 'space-after-tab whitespace-style)
(whitespace-replace-action
- (if whitespace-indent-tabs-mode 'tabify 'untabify)
+ (if indent-tabs-mode 'tabify 'untabify)
rstart rend (whitespace-space-after-tab-regexp) 1))
;; ACTION: replace `tab-width' or more SPACEs by TABs.
((memq 'space-after-tab::tab whitespace-style)
@@ -1531,9 +1516,9 @@ whitespace-cleanup-region
;; by SPACEs.
((memq 'space-before-tab whitespace-style)
(whitespace-replace-action
- (if whitespace-indent-tabs-mode 'tabify 'untabify)
+ (if indent-tabs-mode 'tabify 'untabify)
rstart rend whitespace-space-before-tab-regexp
- (if whitespace-indent-tabs-mode 0 2)))
+ (if indent-tabs-mode 0 2)))
;; ACTION: replace SPACEs before TAB by TABs.
((memq 'space-before-tab::tab whitespace-style)
(whitespace-replace-action
@@ -1564,25 +1549,25 @@ whitespace-replace-action
(defun whitespace-regexp (regexp &optional kind)
- "Return REGEXP depending on `whitespace-indent-tabs-mode'."
+ "Return REGEXP depending on `indent-tabs-mode'."
(format
(cond
((or (eq kind 'tab)
- whitespace-indent-tabs-mode)
+ indent-tabs-mode)
(car regexp))
((or (eq kind 'space)
- (not whitespace-indent-tabs-mode))
+ (not indent-tabs-mode))
(cdr regexp)))
- whitespace-tab-width))
+ tab-width))
(defun whitespace-indentation-regexp (&optional kind)
- "Return the indentation regexp depending on `whitespace-indent-tabs-mode'."
+ "Return the indentation regexp depending on `indent-tabs-mode'."
(whitespace-regexp whitespace-indentation-regexp kind))
(defun whitespace-space-after-tab-regexp (&optional kind)
- "Return the space-after-tab regexp depending on `whitespace-indent-tabs-mode'."
+ "Return the space-after-tab regexp depending on `indent-tabs-mode'."
(whitespace-regexp whitespace-space-after-tab-regexp kind))
@@ -1744,10 +1729,10 @@ whitespace-report-region
whitespace-report-list)))
(when (pcase report-if-bogus (`nil t) (`never nil) (_ has-bogus))
(whitespace-kill-buffer whitespace-report-buffer-name)
- ;; `whitespace-indent-tabs-mode' is local to current buffer
- ;; `whitespace-tab-width' is local to current buffer
- (let ((ws-indent-tabs-mode whitespace-indent-tabs-mode)
- (ws-tab-width whitespace-tab-width))
+ ;; `indent-tabs-mode' may be local to current buffer
+ ;; `tab-width' may be local to current buffer
+ (let ((ws-indent-tabs-mode indent-tabs-mode)
+ (ws-tab-width tab-width))
(with-current-buffer (get-buffer-create
whitespace-report-buffer-name)
(erase-buffer)
@@ -2027,7 +2012,6 @@ whitespace-turn-on
(if (listp whitespace-style)
whitespace-style
(list whitespace-style)))
- (whitespace-ensure-local-variables)
;; turn on whitespace
(when whitespace-active-style
(whitespace-color-on)
@@ -2105,10 +2089,10 @@ whitespace-color-on
`((,(let ((line-column (or whitespace-line-column fill-column)))
(format
"^\\([^\t\n]\\{%s\\}\\|[^\t\n]\\{0,%s\\}\t\\)\\{%d\\}%s\\(.+\\)$"
- whitespace-tab-width
- (1- whitespace-tab-width)
- (/ line-column whitespace-tab-width)
- (let ((rem (% line-column whitespace-tab-width)))
+ tab-width
+ (1- tab-width)
+ (/ line-column tab-width)
+ (let ((rem (% line-column tab-width)))
(if (zerop rem)
""
(format ".\\{%d\\}" rem)))))
@@ -2123,7 +2107,7 @@ whitespace-color-on
,(cond
((memq 'space-before-tab whitespace-active-style)
;; Show SPACEs before TAB (indent-tabs-mode).
- (if whitespace-indent-tabs-mode 1 2))
+ (if indent-tabs-mode 1 2))
((memq 'space-before-tab::tab whitespace-active-style)
1)
((memq 'space-before-tab::space whitespace-active-style)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-21 3:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-02 13:48 bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly Reuben Thomas
2017-03-24 0:06 ` npostavs
2017-04-18 12:06 ` npostavs
2017-04-19 23:25 ` Reuben Thomas
2017-04-21 3:17 ` npostavs
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).