bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 3860 bytes --]

Hi,

I personally have dealt with the issue where I cannot add untabify directly
to write-file-functions hook, because untabify does not return nil.

I need some sort of custom wrapper that runs untabify and then returns nil.

Today I noticed that another user on help-gnu-emacs list faced the same
problem.

That made me submit this minor edit to the untabify function; it simply
returns nil. I noticed that similar edit had to be done for another
function commonly added to write-file-functions: delete-trailing-whitespace.

PATCH follows:

From 1e12773ffa7c94610df070e38aaf8b2315c18fa8 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Tue, 15 Sep 2015 23:24:27 -0400
Subject: [PATCH] Make untabify work with write-file-functions hook

- write-file-functions requires the hooked functions to return nil in
  order to proceed with the file saving.
- So the return value of untabify is set to nil.
---
 lisp/tabify.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/tabify.el b/lisp/tabify.el
index c2f4e0c..9df3eaa 100644
--- a/lisp/tabify.el
+++ b/lisp/tabify.el
@@ -53,7 +53,9 @@ The variable `tab-width' controls the spacing of tab
stops."
             (setq column (current-column))
             (delete-region tab-beg (point))
             (indent-to column)))))
-    (move-to-column c)))
+    (move-to-column c))
+  ;; Return nil for the benefit of `write-file-functions'.
+  nil)

 (defvar tabify-regexp " [ \t]+"
   "Regexp matching whitespace that tabify should consider.
-- 
2.6.0.rc0.24.gec371ff

​Please review and commit to the master branch if this is fine.

For these few lines, I will not need the copyright paperwork, but I would
like to mention that I have already got the copyright paperwork approved
and in place​ (#1029578).


--
Kaushal Modi

On Tue, Sep 15, 2015 at 6:58 PM, warren ferguson <gobold1y@hotmail.com>
wrote:

> You are correct.
> In my .emacs file I too had a detabify function, and when I added nil as
> the return value my saves started working.
> Thanks so much, Warren
>
> ------------------------------
> Date: Tue, 15 Sep 2015 17:55:56 -0400
> Subject: Re: Issues with edited files not being saved
> From: kaushal.modi@gmail.com
> To: gobold1y@hotmail.com
>
>
> Have you added functions to the write-file-functions hook? Or to the old
> hook name that this new name obsoleted?
>
> If so, make sure that all of those functions return nil. To test if one or
> more of the functions added to this hook are a problem, remove all
> functions from the hook, and then saving should work fine.
>
> I had to create a custom untabify function that returned nil to fix this
> issue:
> https://github.com/kaushalmodi/.emacs.d/blob/master/setup-files/setup-editing.el#L99
> On Sep 15, 2015 5:22 PM, "warren ferguson" <gobold1y@hotmail.com> wrote:
>
> I've just downloaded and build a recent version of emacs.
>
> GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
> > head -n 1 /.image
> LinuxSET EC Image SLES11SP2-2 Revision 0 ia32e
> > sysname -afs
> x86-64_linux30
>
> Unfortunately, when I edit a file and try to save it, the status line
> shows the usual "saving" message but that message never goes away
> indicating the save completed. While the attempt to save is ongoing, I can
> go to the terminal window and check that the file has not been updated to
> reflect the edits.
> Interestingly, if I save the edited file to a new unused file name, the
> save does complete.
> How do I determine why emacs is unable to write over the original file?
> I checked file permissions, and they don't seem to be the source of the
> problem. Indeed, the emacs (version 23.2.1) that came with the OS is able
> to save edits to the same file.
>
>
>

[-- Attachment #2: Type: text/html, Size: 9328 bytes --]

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Glenn Morris]

Kaushal Modi wrote:

> I personally have dealt with the issue where I cannot add untabify directly
> to write-file-functions hook, because untabify does not return nil.
>
> I need some sort of custom wrapper that runs untabify and then returns nil.

I'm pretty sure this has come up before.
IMO it's a feature. :)
Simply adding untabify to write-file-functions without thinking about it
(which not consulting the return value indicates) is a recipe for trouble.
Eg hope you like silently breaking every makefile you ever edit,
and irritating any other users of a shared VCS with whitespace changes.

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

> which not consulting the return value indicates

I reviewed the code of untabify and it is not designed to return any value,
so setting the return value to nil does not hurt.

> Eg hope you like silently breaking every makefile you ever edit

I agree. The user needs to use their due diligence when they add untabify
to write-file-functions. They need to know how and when the hooks work and
the exact outcomes of the functions they add to the hooks. So I do NOT add
untabify to this hook globally.

> and irritating any other users of a shared VCS with whitespace changes.

- I am using Verilog code almost 95% of the time. For that, tabs don't
serve any special intent as in the Makefiles (also, in my career, I have
come across Makefile as the only "language" that needs tabs).
- The version control system we use does not care about white space diffs.
- Other team mates would unknowingly use tabs instead of spaces (they
wouldn't have cared if they used tabs or spaces or knew what difference
that makes). But that caused a lot of visual indentation annoyances and
trouble using stuff like multiple cursors. If you have a column of
contiguous white space stretching multiple lines, with some lines having
spaces and some having tabs, you won't be able to get a straight column of
multiple cursors through that.

So in the end, adding untabify locally only for verilog-mode-hook was a
transparent action.. No one realized that untabification was happening and
I also got to improve my coding experience.

That said, if making untabify returning nil does not break its
functionality when using alone or in correspondence with other functions,
is there any opposition to this commit.

As for the "issue" of vcs white space pollution, the user simply should not
add this function to the hook if they care about the whitespace pollution.

--
Kaushal Modi
On Sep 16, 2015 4:24 AM, "Glenn Morris" <rgm@gnu.org> wrote:

> Kaushal Modi wrote:
>
> > I personally have dealt with the issue where I cannot add untabify
> directly
> > to write-file-functions hook, because untabify does not return nil.
> >
> > I need some sort of custom wrapper that runs untabify and then returns
> nil.
>
> I'm pretty sure this has come up before.
> IMO it's a feature. :)
> Simply adding untabify to write-file-functions without thinking about it
> (which not consulting the return value indicates) is a recipe for trouble.
> Eg hope you like silently breaking every makefile you ever edit,
> and irritating any other users of a shared VCS with whitespace changes.
>

[-- Attachment #2: Type: text/html, Size: 3057 bytes --]

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

On another thought, please let me work on an alternative patch that
educates user about untabify. Will update in few hours.
On Sep 16, 2015 8:55 AM, "Kaushal Modi" <kaushal.modi@gmail.com> wrote:

> > which not consulting the return value indicates
>
> I reviewed the code of untabify and it is not designed to return any
> value, so setting the return value to nil does not hurt.
>
> > Eg hope you like silently breaking every makefile you ever edit
>
> I agree. The user needs to use their due diligence when they add untabify
> to write-file-functions. They need to know how and when the hooks work and
> the exact outcomes of the functions they add to the hooks. So I do NOT add
> untabify to this hook globally.
>
> > and irritating any other users of a shared VCS with whitespace changes.
>
> - I am using Verilog code almost 95% of the time. For that, tabs don't
> serve any special intent as in the Makefiles (also, in my career, I have
> come across Makefile as the only "language" that needs tabs).
> - The version control system we use does not care about white space diffs.
> - Other team mates would unknowingly use tabs instead of spaces (they
> wouldn't have cared if they used tabs or spaces or knew what difference
> that makes). But that caused a lot of visual indentation annoyances and
> trouble using stuff like multiple cursors. If you have a column of
> contiguous white space stretching multiple lines, with some lines having
> spaces and some having tabs, you won't be able to get a straight column of
> multiple cursors through that.
>
> So in the end, adding untabify locally only for verilog-mode-hook was a
> transparent action.. No one realized that untabification was happening and
> I also got to improve my coding experience.
>
> That said, if making untabify returning nil does not break its
> functionality when using alone or in correspondence with other functions,
> is there any opposition to this commit.
>
> As for the "issue" of vcs white space pollution, the user simply should
> not add this function to the hook if they care about the whitespace
> pollution.
>
> --
> Kaushal Modi
> On Sep 16, 2015 4:24 AM, "Glenn Morris" <rgm@gnu.org> wrote:
>
>> Kaushal Modi wrote:
>>
>> > I personally have dealt with the issue where I cannot add untabify
>> directly
>> > to write-file-functions hook, because untabify does not return nil.
>> >
>> > I need some sort of custom wrapper that runs untabify and then returns
>> nil.
>>
>> I'm pretty sure this has come up before.
>> IMO it's a feature. :)
>> Simply adding untabify to write-file-functions without thinking about it
>> (which not consulting the return value indicates) is a recipe for trouble.
>> Eg hope you like silently breaking every makefile you ever edit,
>> and irritating any other users of a shared VCS with whitespace changes.
>>
>

[-- Attachment #2: Type: text/html, Size: 3518 bytes --]

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Stefan Monnier]

> I personally have dealt with the issue where I cannot add untabify directly
> to write-file-functions hook, because untabify does not return nil.

If it hurts, then don't do that.  We have before-save-hook for that.


        Stefan

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]

Thanks for the right direction, Stefan.

I am closing this bug as it makes sense to not modify the untabify
function. I was following the example of delete-trailing-whitespace.

For reference, I now have put the crux of this discussion in my config as
below:

(defun modi/untabify-buffer ()
  "Untabify the current buffer.

http://www.veripool.org/issues/345-Verilog-mode-can-t-get-untabify-on-save-to-work
Note that the function's return value is set to nil because if this
function is
added to `write-file-functions' hook, emacs will stay stuck at at the
\"Saving file ..\" message and the file won't be saved if any function
added to
`write-file-functions' returned a non-nil value.

As per the suggestion in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21492
,
for this purpose, it makes a better sense to use `before-save-hook' (a
normal
hook) instead of `write-file-functions' (an abnormal hook that relies on
stuff
like the function return values).

So below would be a recommended way of using this function:

    (defun my/verilog-mode-customizations ()
      (add-hook 'before-save-hook #'modi/untabify-buffer nil :local))
    (add-hook 'verilog-mode-hook #'my/verilog-mode-customizations)

Note that it is suggested to add this function to the `before-save-hook'
*locally* within a hook for a major mode which does not require the use of
tabs instead of spaces. Do NOT add this function to the hook globally,
because it can cause issues with files like Makefiles that rely on the use
of
tabs explicitly."
  (interactive)
  (untabify (point-min) (point-max))
  ;; Return nil for the benefit of `write-file-functions'.
  nil)

I also have now started using before-save-hook instead of
write-file-functions where I don't care about the return values of the
functions added to the hook.

Thanks Glenn, Stefan!


On Wed, Sep 16, 2015 at 9:14 AM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > I personally have dealt with the issue where I cannot add untabify
> directly
> > to write-file-functions hook, because untabify does not return nil.
>
> If it hurts, then don't do that.  We have before-save-hook for that.
>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 3124 bytes --]

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Stefan Monnier]

> I also have now started using before-save-hook instead of
> write-file-functions where I don't care about the return values of the
> functions added to the hook.

BTW, the use of before-save-hook is not just so you don't have to care
about the return value.  It's also because some functions on
write-file-functions may not be run at all (if some earlier function
returns non-nil because it took care of performing the write).

So even adding a "nil" at the of your function doesn't make it right to
add it to write-file-functions.


        Stefan

bug#21492: 25.0.50: Make untabify work nicely with write-file-functions

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

Thanks.

Yes, I had then ended up replacing all my write-file-functions add-hooks
with before-save-hook:
https://github.com/kaushalmodi/.emacs.d/commit/d89d6333f0978d7fda4af412ecaa5089812b7e26

--
Kaushal Modi
On Sep 16, 2015 9:54 PM, "Stefan Monnier" <monnier@iro.umontreal.ca> wrote:

> > I also have now started using before-save-hook instead of
> > write-file-functions where I don't care about the return values of the
> > functions added to the hook.
>
> BTW, the use of before-save-hook is not just so you don't have to care
> about the return value.  It's also because some functions on
> write-file-functions may not be run at all (if some earlier function
> returns non-nil because it took care of performing the write).
>
> So even adding a "nil" at the of your function doesn't make it right to
> add it to write-file-functions.
>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 1347 bytes --]