bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

Reuben Thomas
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.

--

0001-Fix-reading-of-tab-settings-in-whitespace-mode.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

Noam Postavsky-2
Reuben Thomas <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

Noam Postavsky-2
[hidden email] writes:

> Reuben Thomas <[hidden email]> 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’




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

Reuben Thomas
On 18 April 2017 at 13:06,  <[hidden email]> wrote:
> [hidden email] 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

0006-Fix-reading-of-tab-settings-in-whitespace-mode.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#25936: Fix for whitespace.el to make it read tab-width and indent-tabs-mode correctly

Noam Postavsky-2
tags 25936 fixed
close 25936 26.1
quit

Reuben Thomas <[hidden email]> writes:

> On 18 April 2017 at 13:06,  <[hidden email]> wrote:
>
> Thanks very much for this. Revised patch attached.

Thanks, pushed to master [1: a6b375ba4b].

[1: a6b375ba4b]: 2017-04-20 22:39:15 -0400
  Fix reading of tab settings in whitespace-mode
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=a6b375ba4bfc9453abc428dcb73e65bfcf61b794



Loading...