bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Björn Lindström]

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

Hello,

attaching patch to make the whitespace-cleanup and whitespace-cleanup-region functions add a final newline to a file if whitespace-style contains `missing-newline-at-eof

I'm aware this somewhat replicates what setting `require-final-newline would do, but I think since whitespace.el with this configuration highlights this as an error, it should also clean it up when asked.

Best wishes,
Björn

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-whitespace.el-cleanup-add-missing-final-newline.patch --]
[-- Type: text/x-patch; name="0001-Make-whitespace.el-cleanup-add-missing-final-newline.patch", Size: 1749 bytes --]

From 73e20b34c1669a9d52cbc70f4c02cc26fa23cf5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Lindstr=C3=B6m?= <bkhl@elektrubadur.se>
Date: Tue, 11 Jun 2024 19:49:55 +0200
Subject: [PATCH] Make whitespace.el cleanup add missing final newline

* lisp/whitespace.el (whitespace-cleanup-region): if cleaning up at end
of file, add missing newline if indicated by whitespace-style.
---
 lisp/whitespace.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index bc23a8794eb..a41a7520915 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1465,6 +1465,11 @@ defun whitespace-cleanup-region
    If `whitespace-style' includes the value
    `space-after-tab::space', replace TABs by SPACEs.
 
+5. missing newline at end of file.
+   If `whitespace-style' includes the value `missing-newline-at-eof',
+   and the cleanup region includes the end of file, add a final newline
+   if it is not there already.
+
 See `whitespace-style', `indent-tabs-mode' and `tab-width' for
 documentation."
   (interactive "@r")
@@ -1545,7 +1550,13 @@ defun whitespace-cleanup-region
          ((memq 'space-before-tab::space whitespace-style)
           (whitespace-replace-action
            'untabify rstart rend
-           whitespace-space-before-tab-regexp 2))))
+           whitespace-space-before-tab-regexp 2)))
+        ;; PROBLEM 5: missing newline at end of file
+        (when (and (memq 'missing-newline-at-eof)
+                   (= (point-max) (without-restriction (point-max))))
+          (goto-char (point-max))
+          (when (re-search-backward ".\\'" nil t)
+            (insert "\n")))
       (set-marker rend nil))))		; point marker to nowhere
 
 
-- 
2.45.2

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Björn Lindström]

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

On Tue, Jun 11, 2024, at 20:16, Björn Lindström wrote:
> Hello,
>
> attaching patch to make the whitespace-cleanup and 
> whitespace-cleanup-region functions add a final newline to a file if 
> whitespace-style contains `missing-newline-at-eof
>
> I'm aware this somewhat replicates what setting `require-final-newline 
> would do, but I think since whitespace.el with this configuration 
> highlights this as an error, it should also clean it up when asked.
>
> Best wishes,
> Björn
> Attachments:
> * 0001-Make-whitespace.el-cleanup-add-missing-final-newline.patch

Sorry, somehow messed up the first patch I sent, now attaching a corrected one. Attaching the correct one here.

Best wishes,
Björn

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-whitespace.el-cleanup-add-missing-final-newline-2.patch --]
[-- Type: text/x-patch; name="0001-Make-whitespace.el-cleanup-add-missing-final-newline-2.patch", Size: 1805 bytes --]

From 3e180674604f41de70198c3aaa3b0bc5cddf0a68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Lindstr=C3=B6m?= <bkhl@elektrubadur.se>
Date: Tue, 11 Jun 2024 19:49:55 +0200
Subject: [PATCH] Make whitespace.el cleanup add missing final newline

* lisp/whitespace.el (whitespace-cleanup-region): if cleaning up at end
of file, add missing newline if indicated by whitespace-style.
---
 lisp/whitespace.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index bc23a8794eb..6f8bd0b8585 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1465,6 +1465,11 @@ defun whitespace-cleanup-region
    If `whitespace-style' includes the value
    `space-after-tab::space', replace TABs by SPACEs.
 
+5. missing newline at end of file.
+   If `whitespace-style' includes the value `missing-newline-at-eof',
+   and the cleanup region includes the end of file, add a final newline
+   if it is not there already.
+
 See `whitespace-style', `indent-tabs-mode' and `tab-width' for
 documentation."
   (interactive "@r")
@@ -1545,7 +1550,14 @@ defun whitespace-cleanup-region
          ((memq 'space-before-tab::space whitespace-style)
           (whitespace-replace-action
            'untabify rstart rend
-           whitespace-space-before-tab-regexp 2))))
+           whitespace-space-before-tab-regexp 2)))
+        ;; PROBLEM 5: missing newline at end of file
+        (when (and (memq 'missing-newline-at-eof whitespace-style)
+                   (= (point-max) (without-restriction (point-max))))
+          (goto-char (point-max))
+          (when (re-search-backward ".\\'" nil t)
+            (goto-char (point-max))
+            (insert "\n"))))
       (set-marker rend nil))))		; point marker to nowhere
 
 
-- 
2.45.2

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> Date: Tue, 11 Jun 2024 20:16:03 +0200
> From: Björn Lindström <bkhl@elektrubadur.se>
> 
> attaching patch to make the whitespace-cleanup and whitespace-cleanup-region functions add a final newline to a file if whitespace-style contains `missing-newline-at-eof
> 
> I'm aware this somewhat replicates what setting `require-final-newline would do, but I think since whitespace.el with this configuration highlights this as an error, it should also clean it up when asked.

This is an incompatible change of behavior on behalf of
whitespace-cleanup, so I don't think we can accept it as in this patch
(or the next one you sent).  Se we'd need some new user option, by
default off, to turn on this new feature.  Or maybe a new action for
whitespace-action?

Thanks.

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Björn Lindström]

On Wed, Jun 12, 2024, at 09:46, Eli Zaretskii wrote:
>> Date: Tue, 11 Jun 2024 20:16:03 +0200
>> From: Björn Lindström <bkhl@elektrubadur.se>
>> 
>> attaching patch to make the whitespace-cleanup and whitespace-cleanup-region functions add a final newline to a file if whitespace-style contains `missing-newline-at-eof
>> 
>> I'm aware this somewhat replicates what setting `require-final-newline would do, but I think since whitespace.el with this configuration highlights this as an error, it should also clean it up when asked.
>
> This is an incompatible change of behavior on behalf of
> whitespace-cleanup, so I don't think we can accept it as in this patch
> (or the next one you sent).  Se we'd need some new user option, by
> default off, to turn on this new feature.  Or maybe a new action for
> whitespace-action?
>
> Thanks.

Hello,

I thought about that, but since whitespace-cleanup generally applies clean-up according to white-space style, I thought it was simply an oversight that it doesn't apply a fix when it is set to highlight missing end-of-file newline.

Adding a separate way to configure this removes the simplicity of configuring your preferred whitespace-style as a single option.

However, If you still disagree I can make another patch somehow maintains the old behaviour as the default, so just let me know.

Thanks,
Björn

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> Date: Wed, 12 Jun 2024 11:04:18 +0200
> From: Björn Lindström <bkhl@elektrubadur.se>
> Cc: 71499@debbugs.gnu.org
> 
> On Wed, Jun 12, 2024, at 09:46, Eli Zaretskii wrote:
> >> Date: Tue, 11 Jun 2024 20:16:03 +0200
> >> From: Björn Lindström <bkhl@elektrubadur.se>
> >> 
> > This is an incompatible change of behavior on behalf of
> > whitespace-cleanup, so I don't think we can accept it as in this patch
> > (or the next one you sent).  Se we'd need some new user option, by
> > default off, to turn on this new feature.  Or maybe a new action for
> > whitespace-action?
> >
> > Thanks.
> 
> Hello,
> 
> I thought about that, but since whitespace-cleanup generally applies clean-up according to white-space style, I thought it was simply an oversight that it doesn't apply a fix when it is set to highlight missing end-of-file newline.
> 
> Adding a separate way to configure this removes the simplicity of configuring your preferred whitespace-style as a single option.
> 
> However, If you still disagree I can make another patch somehow maintains the old behaviour as the default, so just let me know.

Let's see what others think, and take it from there.

Stefan, Andrea: WDYT about this change?

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> I thought about that, but since whitespace-cleanup generally applies
>> clean-up according to white-space style, I thought it was simply an
>> oversight that it doesn't apply a fix when it is set to highlight
>> missing end-of-file newline.
>>
>> Adding a separate way to configure this removes the simplicity of
>> configuring your preferred whitespace-style as a single option.
>>
>> However, If you still disagree I can make another patch somehow
>> maintains the old behaviour as the default, so just let me know.
>
> Let's see what others think, and take it from there.
>
> Stefan, Andrea: WDYT about this change?

AFAIU, the purpose of whitespace.el is to detect and eventually fix
incorrect whitespace, and it has two ways of doing this:

- Visual highlighting
- Commands to fix problems (`whitespace-report` and
  `whitespace-cleanup).

Since it is mostly configured in the centralized option,
`whitespace-style`, it seems natural that if a user wants to detect
`missing-newline-at-eof`, she would also want this to be fixed by
`whitespace-cleanup`.  This seems even more natural given that
`whitespace-report` already considers that a problem worthy of
reporting.  IOW, I tend to agree that this not already being the case
looks like an oversight.

So I think the existing options are fine, and the patch could go in
as-is, despite the fact that it is backwards-incompatible.  If users
really hate it, I guess we will hear about it and can react.

If we want to be really cautious, we might want to consider waiting with
this change until Emacs 31.  That should provide ample time for people
to notice the new behaviour and react.

My two cents.

BTW, note that we have `require-final-newline` as well, which is what I
use here (in combination with `delete-trailing-whitespace` and the
third-party package `ws-butler`).

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Andrea Corallo]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> I thought about that, but since whitespace-cleanup generally applies
>>> clean-up according to white-space style, I thought it was simply an
>>> oversight that it doesn't apply a fix when it is set to highlight
>>> missing end-of-file newline.
>>>
>>> Adding a separate way to configure this removes the simplicity of
>>> configuring your preferred whitespace-style as a single option.
>>>
>>> However, If you still disagree I can make another patch somehow
>>> maintains the old behaviour as the default, so just let me know.
>>
>> Let's see what others think, and take it from there.
>>
>> Stefan, Andrea: WDYT about this change?
>
> AFAIU, the purpose of whitespace.el is to detect and eventually fix
> incorrect whitespace, and it has two ways of doing this:
>
> - Visual highlighting
> - Commands to fix problems (`whitespace-report` and
>   `whitespace-cleanup).
>
> Since it is mostly configured in the centralized option,
> `whitespace-style`, it seems natural that if a user wants to detect
> `missing-newline-at-eof`, she would also want this to be fixed by
> `whitespace-cleanup`.  This seems even more natural given that
> `whitespace-report` already considers that a problem worthy of
> reporting.  IOW, I tend to agree that this not already being the case
> looks like an oversight.
>
> So I think the existing options are fine, and the patch could go in
> as-is, despite the fact that it is backwards-incompatible.  If users
> really hate it, I guess we will hear about it and can react.
>
> If we want to be really cautious, we might want to consider waiting with
> this change until Emacs 31.  That should provide ample time for people
> to notice the new behaviour and react.
>
> My two cents.

I'm as well for having the patch in, but I guess would be safer in 31 so
we have plenty of time to react if needed.

  Andrea

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> From: Andrea Corallo <acorallo@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Björn Lindström
>  <bkhl@elektrubadur.se>,
>   71499@debbugs.gnu.org
> Date: Thu, 13 Jun 2024 03:38:42 -0400
> 
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> I thought about that, but since whitespace-cleanup generally applies
> >>> clean-up according to white-space style, I thought it was simply an
> >>> oversight that it doesn't apply a fix when it is set to highlight
> >>> missing end-of-file newline.
> >>>
> >>> Adding a separate way to configure this removes the simplicity of
> >>> configuring your preferred whitespace-style as a single option.
> >>>
> >>> However, If you still disagree I can make another patch somehow
> >>> maintains the old behaviour as the default, so just let me know.
> >>
> >> Let's see what others think, and take it from there.
> >>
> >> Stefan, Andrea: WDYT about this change?
> >
> > AFAIU, the purpose of whitespace.el is to detect and eventually fix
> > incorrect whitespace, and it has two ways of doing this:
> >
> > - Visual highlighting
> > - Commands to fix problems (`whitespace-report` and
> >   `whitespace-cleanup).
> >
> > Since it is mostly configured in the centralized option,
> > `whitespace-style`, it seems natural that if a user wants to detect
> > `missing-newline-at-eof`, she would also want this to be fixed by
> > `whitespace-cleanup`.  This seems even more natural given that
> > `whitespace-report` already considers that a problem worthy of
> > reporting.  IOW, I tend to agree that this not already being the case
> > looks like an oversight.
> >
> > So I think the existing options are fine, and the patch could go in
> > as-is, despite the fact that it is backwards-incompatible.  If users
> > really hate it, I guess we will hear about it and can react.
> >
> > If we want to be really cautious, we might want to consider waiting with
> > this change until Emacs 31.  That should provide ample time for people
> > to notice the new behaviour and react.
> >
> > My two cents.
> 
> I'm as well for having the patch in, but I guess would be safer in 31 so
> we have plenty of time to react if needed.

OK, thanks.  I will then install after the branch is cut.

Meanwhile, Björn, I have a few comments to the patch:

  . it needs a NEWS entry announcing the new feature
  . is there any reason your code to handle the missing newline is not
    identical to what the implementation of require-final-newline
    does?

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Robert Pluim]

>>>>> On Thu, 13 Jun 2024 11:30:51 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> I'm as well for having the patch in, but I guess would be safer in 31 so
    >> we have plenty of time to react if needed.

    Eli> OK, thanks.  I will then install after the branch is cut.

    Eli> Meanwhile, Björn, I have a few comments to the patch:

    Eli>   . it needs a NEWS entry announcing the new feature
    Eli>   . is there any reason your code to handle the missing newline is not
    Eli>     identical to what the implementation of require-final-newline
    Eli>     does?

Iʼm going to chime in and say that, even though I highlight missing
newlines at eob, I donʼt normally set `require-final-newline' to t,
because of various reasons involving collaboration with others. So Iʼd
have to turn this off once it goes in.

Robert
-- 

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Andrea Corallo <acorallo@gnu.org>,  71499@debbugs.gnu.org,
>   stefankangas@gmail.com,  bkhl@elektrubadur.se
> Date: Fri, 14 Jun 2024 14:23:17 +0200
> 
> >>>>> On Thu, 13 Jun 2024 11:30:51 +0300, Eli Zaretskii <eliz@gnu.org> said:
>     >> I'm as well for having the patch in, but I guess would be safer in 31 so
>     >> we have plenty of time to react if needed.
> 
>     Eli> OK, thanks.  I will then install after the branch is cut.
> 
>     Eli> Meanwhile, Björn, I have a few comments to the patch:
> 
>     Eli>   . it needs a NEWS entry announcing the new feature
>     Eli>   . is there any reason your code to handle the missing newline is not
>     Eli>     identical to what the implementation of require-final-newline
>     Eli>     does?
> 
> Iʼm going to chime in and say that, even though I highlight missing
> newlines at eob, I donʼt normally set `require-final-newline' to t,
> because of various reasons involving collaboration with others. So Iʼd
> have to turn this off once it goes in.

Maybe this feature should take a hint from require-final-newline?

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Andrea Corallo <acorallo@gnu.org>,  71499@debbugs.gnu.org,
>>   stefankangas@gmail.com,  bkhl@elektrubadur.se
>> Date: Fri, 14 Jun 2024 14:23:17 +0200
>>
>> >>>>> On Thu, 13 Jun 2024 11:30:51 +0300, Eli Zaretskii <eliz@gnu.org> said:
>>     >> I'm as well for having the patch in, but I guess would be safer in 31 so
>>     >> we have plenty of time to react if needed.
>>
>>     Eli> OK, thanks.  I will then install after the branch is cut.
>>
>>     Eli> Meanwhile, Björn, I have a few comments to the patch:
>>
>>     Eli>   . it needs a NEWS entry announcing the new feature
>>     Eli>   . is there any reason your code to handle the missing newline is not
>>     Eli>     identical to what the implementation of require-final-newline
>>     Eli>     does?
>>
>> Iʼm going to chime in and say that, even though I highlight missing
>> newlines at eob, I donʼt normally set `require-final-newline' to t,
>> because of various reasons involving collaboration with others. So Iʼd
>> have to turn this off once it goes in.

Could you explain why you'd need to turn it off?

Do you usually use `whitespace-cleanup` on files where you collaborate
with others, but then you specifically don't want to ever touch newlines
at eob?

> Maybe this feature should take a hint from require-final-newline?

To my mind, the main use case of this feature would be users that don't
want to set `require-final-newline`, instead preferring to make such
fixes manually.

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Robert Pluim]

>>>>> On Thu, 20 Jun 2024 07:55:19 +0000, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Eli Zaretskii <eliz@gnu.org> writes:
    >>> From: Robert Pluim <rpluim@gmail.com>
    >>> Cc: Andrea Corallo <acorallo@gnu.org>,  71499@debbugs.gnu.org,
    >>> stefankangas@gmail.com,  bkhl@elektrubadur.se
    >>> Date: Fri, 14 Jun 2024 14:23:17 +0200
    >>> 
    >>> >>>>> On Thu, 13 Jun 2024 11:30:51 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >>> >> I'm as well for having the patch in, but I guess would be safer in 31 so
    >>> >> we have plenty of time to react if needed.
    >>> 
    Eli> OK, thanks.  I will then install after the branch is cut.
    >>> 
    Eli> Meanwhile, Björn, I have a few comments to the patch:
    >>> 
    Eli> . it needs a NEWS entry announcing the new feature
    Eli> . is there any reason your code to handle the missing newline is not
    Eli> identical to what the implementation of require-final-newline
    Eli> does?
    >>> 
    >>> Iʼm going to chime in and say that, even though I highlight missing
    >>> newlines at eob, I donʼt normally set `require-final-newline' to t,
    >>> because of various reasons involving collaboration with others. So Iʼd
    >>> have to turn this off once it goes in.

    Stefan> Could you explain why you'd need to turn it off?

    Stefan> Do you usually use `whitespace-cleanup` on files where you collaborate
    Stefan> with others, but then you specifically don't want to ever touch newlines
    Stefan> at eob?

Yes. I guess I could turn off whitespace-cleanup for those files, and
complain at those people more loudly :-) . I donʼt have a strong
objection to this feature, so we donʼt have to add
yet-another-config-variable just for me.

Robert
-- 

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> Yes. I guess I could turn off whitespace-cleanup for those files, and
> complain at those people more loudly :-) .

Just to confirm: you still want it highlighted, despite the fact that
you're anyways not planning to change it?  IOW, you couldn't just
customize `whitespace-style` in those directories and have it do what
you want?

Perhaps we should have `whitespace-disable-cleanups` for these use
cases, but it should then allow customizing a list of values similar to
the ones in `whitespace-style`.

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Robert Pluim]

>>>>> On Thu, 20 Jun 2024 08:55:04 +0000, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Robert Pluim <rpluim@gmail.com> writes:
    >> Yes. I guess I could turn off whitespace-cleanup for those files, and
    >> complain at those people more loudly :-) .

    Stefan> Just to confirm: you still want it highlighted, despite the fact that
    Stefan> you're anyways not planning to change it?  IOW, you couldn't just
    Stefan> customize `whitespace-style` in those directories and have it do what
    Stefan> you want?

Thatʼs another way I could fix it, but itʼs fixable just in my config.

    Stefan> Perhaps we should have `whitespace-disable-cleanups` for these use
    Stefan> cases, but it should then allow customizing a list of values similar to
    Stefan> the ones in `whitespace-style`.

Again: letʼs not overcomplicate this just for my niche case.

Robert
-- 

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> Thatʼs another way I could fix it, but itʼs fixable just in my config.
[...]
> Again: letʼs not overcomplicate this just for my niche case.

Sounds good to me.

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> Cc: 71499@debbugs.gnu.org, stefankangas@gmail.com, bkhl@elektrubadur.se
> Date: Thu, 13 Jun 2024 11:30:51 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Meanwhile, Björn, I have a few comments to the patch:
> 
>   . it needs a NEWS entry announcing the new feature
>   . is there any reason your code to handle the missing newline is not
>     identical to what the implementation of require-final-newline
>     does?

Ping!  Björn, can you please post an updated patch with the above nits
taken care of?  We can install this now on the master branch.

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Björn Lindström]

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

Hello,

updated patch attached, which ads a note to NEWS, and has a changed implementation of the insertion of missing newline inspired by the one from files.el. It's still slightly different to account for it also being possible to use on a region.

This time there's also an added test.

/ Björn

On Thu, Jun 27, 2024, at 09:37, Eli Zaretskii wrote:
>> Cc: 71499@debbugs.gnu.org, stefankangas@gmail.com, bkhl@elektrubadur.se
>> Date: Thu, 13 Jun 2024 11:30:51 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> Meanwhile, Björn, I have a few comments to the patch:
>> 
>>   . it needs a NEWS entry announcing the new feature
>>   . is there any reason your code to handle the missing newline is not
>>     identical to what the implementation of require-final-newline
>>     does?
>
> Ping!  Björn, can you please post an updated patch with the above nits
> taken care of?  We can install this now on the master branch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-whitespace.el-cleanup-add-missing-final-newline.patch --]
[-- Type: text/x-patch; name="0001-Make-whitespace.el-cleanup-add-missing-final-newline.patch", Size: 4055 bytes --]

From a6ca7984df3eec80e8a36ded73d068073d9a5a0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Lindstr=C3=B6m?= <bkhl@elektrubadur.se>
Date: Tue, 11 Jun 2024 19:49:55 +0200
Subject: [PATCH] Make whitespace.el cleanup add missing final newline

* lisp/whitespace.el (whitespace-cleanup-region): if cleaning up at end
of file, add missing newline if indicated by whitespace-style.
---
 etc/NEWS                      |  6 ++++++
 lisp/whitespace.el            | 16 +++++++++++++++-
 test/lisp/whitespace-tests.el | 15 ++++++++++++++-
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index af32a93d9c4..9f61a4aa4ce 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -36,6 +36,12 @@ applies, and please also update docstrings as needed.
 \f
 * Changes in Specialized Modes and Packages in Emacs 31.1
 
+---
+** Whitespace
+'whitespace-cleanup' now adds missing newline at end of file If
+'whitespace-style' includes 'missing-newline-at-eof (which is the
+default), the 'whitespace-cleanup' function will fix this when run.
+
 \f
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index bc23a8794eb..28d131b054c 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1465,6 +1465,11 @@ defun whitespace-cleanup-region
    If `whitespace-style' includes the value
    `space-after-tab::space', replace TABs by SPACEs.
 
+5. missing newline at end of file.
+   If `whitespace-style' includes the value `missing-newline-at-eof',
+   and the cleanup region includes the end of file, add a final newline
+   if it is not there already.
+
 See `whitespace-style', `indent-tabs-mode' and `tab-width' for
 documentation."
   (interactive "@r")
@@ -1545,7 +1550,16 @@ defun whitespace-cleanup-region
          ((memq 'space-before-tab::space whitespace-style)
           (whitespace-replace-action
            'untabify rstart rend
-           whitespace-space-before-tab-regexp 2))))
+           whitespace-space-before-tab-regexp 2)))
+        ;; PROBLEM 5: missing newline at end of file
+        (and (memq 'missing-newline-at-eof whitespace-style)
+             (> (point-max) (point-min))
+             (= (point-max) (without-restriction (point-max)))
+             (/= (char-before (point-max)) ?\n)
+             (not (and (eq selective-display t)
+                       (= (char-before (point-max)) ?\r)))
+             (goto-char (point-max))
+             (ignore-errors (insert "\n"))))
       (set-marker rend nil))))		; point marker to nowhere
 
 
diff --git a/test/lisp/whitespace-tests.el b/test/lisp/whitespace-tests.el
index 73c7e742ec5..bd35b3ac9f3 100644
--- a/test/lisp/whitespace-tests.el
+++ b/test/lisp/whitespace-tests.el
@@ -8,7 +8,6 @@
 ;; it under the terms of the GNU General Public License as published by
 ;; the Free Software Foundation, either version 3 of the License, or
 ;; (at your option) any later version.
-
 ;; GNU Emacs is distributed in the hope that it will be useful,
 ;; but WITHOUT ANY WARRANTY; without even the implied warranty of
 ;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ -94,6 +93,20 @@ defun whitespace-tests--cleanup-string
     (should (equal (whitespace-tests--cleanup-string "a  \n\t \n\n")
                    "a  \n"))))
 
+(ert-deftest whitespace-cleanup-missing-newline-at-eof ()
+  (let ((whitespace-style '(empty missing-newline-at-eof)))
+    (should (equal (whitespace-tests--cleanup-string "")
+                   ""))
+    (should (equal (whitespace-tests--cleanup-string "a")
+                   "a\n"))
+    (should (equal (whitespace-tests--cleanup-string "a\n\t")
+                   "a\n"))
+    (should (equal (whitespace-tests--cleanup-string "a\n\t ")
+                   "a\n"))
+    (should (equal (whitespace-tests--cleanup-string "a\n\t ")
+                   "a\n"))
+    (should (equal (whitespace-tests--cleanup-string "\n\t")
+                   ""))))
 
 ;; We cannot call whitespace-mode because it will do nothing in batch
 ;; mode.  So we call its innards instead.
-- 
2.45.2

bug#71499: [PATCH] Make whitespace.el cleanup add missing final newline

[Eli Zaretskii]

> Date: Sat, 29 Jun 2024 13:59:03 +0200
> From: Björn Lindström <bkhl@elektrubadur.se>
> 
> updated patch attached, which ads a note to NEWS, and has a changed implementation of the insertion of missing newline inspired by the one from files.el. It's still slightly different to account for it also being possible to use on a region.
> 
> This time there's also an added test.

Thanks, installed on the master branch, and closing the bug.