Re: [PATCH] Unit tests and lexical-binding for delim-col.el

[Basil L. Contovounesios]

Stefan Kangas <address@hidden> writes:

> Basil L. Contovounesios <address@hidden> writes:
>> if no-one objects within the next few days, I or someone else will push the
>> patch to master.
>
> Great.
>
>> I noticed an additional possible cleanup:
>
> I've included these cleanups in the attached patch.
>
> Could we also get rid of this comment?  I don't see how it helps since this
> has been in vanilla for 20 years now.
>
> ;; To use it, make sure that this file is in load-path and insert in your
> ;; .emacs:
> ;;
> ;;    (require 'delim-col)

Sure.  I added that, a link to this discussion, and a missing full stop
to your first patch, refilled one of its long lines, and pushed to
master[1].

[1: 4498e5a13a]: Use lexical-binding in delim-col.el and add tests
  2019-05-20 15:29:26 +0100
  
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4498e5a13a3b63a3024ceef102ae3b5c50f58be1

> Stefan Monnier <address@hidden> writes:
>> BTW, it would be nice to make it so that delimit-columns-region
>> delegates to delimit-columns-rectangle when called with a rectangular
>> region (i.e. using rectangle-mark-mode).
>
> Good idea.  How about the second patch attached here?

Looks fine to me, except for a minor question:

> From 5b2ef61a06001c48a414df3c63f2588c18038f90 Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <address@hidden>
> Date: Sat, 11 May 2019 12:17:18 +0200
> Subject: [PATCH 2/2] Delegate to rectangle version in delim-col when
>  appropriate
>
> * lisp/delim-col.el (delimit-columns-region): Delegate to
> delimit-columns-rectangle when called with a rectangular region.
> ---
>  etc/NEWS          |  6 ++++
>  lisp/delim-col.el | 92 
> +++++++++++++++++++++++++++++--------------------------
>  2 files changed, 54 insertions(+), 44 deletions(-)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index d10a553244..fb70de7902 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1422,6 +1422,12 @@ of an idle Emacs, but may fail on some network file 
> systems; set
>  notification is not supported.  The new variable currently has no
>  effect in 'global-auto-revert-mode'.  The default value is nil.
>  
> +** delim-col
> +
> +*** 'delimit-columns-region' now delegates to
> +'delimit-columns-rectangle' when called with a rectangular region
> +(i.e. using rectangle-mark-mode).
> +
>  
>  * New Modes and Packages in Emacs 27.1

I'm personally fine with this, but I think NEWS entries usually comprise
a summary on the first line, followed by an elaboration in the body.
I'm not very good at writing these, but here's one way to do that:

diff --git a/etc/NEWS b/etc/NEWS
index d4638c17b1..1606d5b50e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1460,9 +1460,10 @@ buffer periodically when 'auto-revert-avoid-polling' is 
non-nil.
 
 ** delim-col
 
-*** 'delimit-columns-region' now delegates to
-'delimit-columns-rectangle' when called with a rectangular region
-(i.e. using rectangle-mark-mode).
+*** 'delimit-columns-region' acts differently on rectangular regions.
+When called with a noncontiguous rectangular region, i.e., when
+rectangle-mark-mode is enabled, 'delimit-columns-region' now acts like
+'delimit-columns-rectangle'.
 
 
 * New Modes and Packages in Emacs 27.1

Here's another possibility to reuse an existing NEWS entry under
"Editing Changes":

diff --git a/etc/NEWS b/etc/NEWS
index d4638c17b1..e1213a036a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -407,7 +407,9 @@ interface that's more like functions like 'search-forward'.
 
 ---
 ** More commands support noncontiguous rectangular regions, namely
-'upcase-dwim', 'downcase-dwim', 'replace-string', 'replace-regexp'.
+'upcase-dwim', 'downcase-dwim', 'replace-string', 'replace-regexp',
+and 'delimit-columns-region'.  The latter delegates to
+'delimit-columns-rectangle' when the region is rectangular.
 
 +++
 ** When asked to visit a large file, Emacs now offers visiting it literally.
@@ -1458,12 +1460,6 @@ the new variable 'buffer-auto-revert-by-notification' to 
a non-nil
 value.  Auto Revert mode can use this information to avoid polling the
 buffer periodically when 'auto-revert-avoid-polling' is non-nil.
 
-** delim-col
-
-*** 'delimit-columns-region' now delegates to
-'delimit-columns-rectangle' when called with a rectangular region
-(i.e. using rectangle-mark-mode).
-
 
 * New Modes and Packages in Emacs 27.1
 

As soon as someone confirms which format is preferred or suggests a
better wording, I'll push the second patch as well.

Thanks,

-- 
Basil