bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Pål Grønås Drange]

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

downcase-region in marked rectangle does not downcase entire rectangle
and downcases things not marked:

On the following content in my buffer (*scratch*), upon marking the
inner rectangle consisting of "FG" and "JK" using C-return (cua-mode
enabled)

    ABCD
    EFGH
    IJKL
    MNOP

Applying `downcase-region` yields the following unexpected result:

    ABCD
    Efgh
    ijKL
    MNOP

I would expect "fg" and "jk" to be downcased, and not "fgh" and "ij".

Original report at http://emacs.stackexchange.com/q/9913/5154

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

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

While this is not the same bug as bug#19829, it's fundamentally the same
issue, and they should be fixed in the same way.


        Stefan


>>>>> "Pål" == Pål Grønås Drange <paal.drange@gmail.com> writes:

> downcase-region in marked rectangle does not downcase entire rectangle
> and downcases things not marked:

> On the following content in my buffer (*scratch*), upon marking the
> inner rectangle consisting of "FG" and "JK" using C-return (cua-mode
> enabled)

>     ABCD
>     EFGH
>     IJKL
>     MNOP

> Applying `downcase-region` yields the following unexpected result:

>     ABCD
>     Efgh
>     ijKL
>     MNOP

> I would expect "fg" and "jk" to be downcased, and not "fgh" and "ij".

> Original report at http://emacs.stackexchange.com/q/9913/5154

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

> While this is not the same bug as bug#19829, it's fundamentally the same
> issue, and they should be fixed in the same way.

Currently, `downcase-region' is defined with the interactive spec 'r'
that calls the function with point and mark as 2 args.

For backward-compatibility reasons this can't be changed.  So maybe
a new interactive spec e.g. 'R' should call the function with 2 lists
where the first list will contain a list of beginnings of all intervals
in the rectangular region, and the second list of endings of intervals?

Then a command using it will need to add another outermost loop to pop
interval boundaries from both list arguments `begs' and `ends' to process
intervals using the same logic like it processes a single region now.

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

> Currently, `downcase-region' is defined with the interactive spec 'r'
> that calls the function with point and mark as 2 args.

Indeed, same problem as kill-region, to which I had to add a `region'
argument (which basically says "we're called interactively so use
whatever is the region rather than BEG and END").

> For backward-compatibility reasons this can't be changed.  So maybe
> a new interactive spec e.g. 'R' should call the function with 2 lists
> where the first list will contain a list of beginnings of all intervals
> in the rectangular region, and the second list of endings of intervals?

Obviously, there will be many more commands that need to be adjusted to
the extended notion of "region" which can now be composed of various extents.
I'm not sure we need/want a new "R", because I think all cases of "r"
are currently broken when the region is non contiguous (i.e. they all
need to use "the new system").

One way to handle it generically, is to change call-interactively such
that when the argument is "r", the command is called once for each
"extent".  Obviously, this won't work correctly for all cases, tho.

Another option is to make "r" behave along the lines of your "R" (tho
I much prefer a list of pairs than a pair of lists, so one of the two
args would end up "useless").

> Then a command using it will need to add another outermost loop to pop
> interval boundaries from both list arguments `begs' and `ends' to process
> intervals using the same logic like it processes a single region now.

Indeed.  Maybe such a loop would be common enough that it'd be
worthwhile adding some ad-hoc handling (e.g. make call-interactively do
the looping as suggested above, tho maybe only after checking some
special property of the command).


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

> Indeed, same problem as kill-region, to which I had to add a `region'
> argument (which basically says "we're called interactively so use
> whatever is the region rather than BEG and END").

Then maybe support for the rectangular region should be introduced
to every command by adding a new argument `region' with a list of pairs
((BEG1 . END1) (BEG2 . END2) ...) and keeping the existing arguments
BEG and END unchanged?

> Obviously, there will be many more commands that need to be adjusted to
> the extended notion of "region" which can now be composed of various extents.
> I'm not sure we need/want a new "R", because I think all cases of "r"
> are currently broken when the region is non contiguous (i.e. they all
> need to use "the new system").
>
> One way to handle it generically, is to change call-interactively such
> that when the argument is "r", the command is called once for each
> "extent".  Obviously, this won't work correctly for all cases, tho.

Also this won't work for commands that use region-beginning/region-end
in the interactive spec (like query-replace for bug#13178/bug#19829).

So while fixing the commands one by one, after fixing every command
it needs to be marked as "supporting the rectangular region".  If the
command used region-beginning and region-end, this means changing them to
e.g. plural region-beginnings and region-ends.

> Another option is to make "r" behave along the lines of your "R" (tho
> I much prefer a list of pairs than a pair of lists, so one of the two
> args would end up "useless").

Yes, the worst part of this feature is that commands should keep
two existing arguments BEG and END, and either add a new meaning to them,
(while supporting their current values as numbers for backward-compatibility),
or add a new optional argument.

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 15 Mar 2015 22:34:02 +0200
> Cc: Pål Grønås Drange
> 	<paal.drange@gmail.com>, 20070@debbugs.gnu.org
> 
> > Indeed, same problem as kill-region, to which I had to add a `region'
> > argument (which basically says "we're called interactively so use
> > whatever is the region rather than BEG and END").
> 
> Then maybe support for the rectangular region should be introduced
> to every command by adding a new argument `region' with a list of pairs
> ((BEG1 . END1) (BEG2 . END2) ...) and keeping the existing arguments
> BEG and END unchanged?

What are BEG1, END1, BEG2, END2, etc. values here?  In what units are
they measured?  Are they pixels, are they columns, something else?

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Nicolas Richard]

Eli Zaretskii <eliz@gnu.org> writes:
> What are BEG1, END1, BEG2, END2, etc. values here?  In what units are
> they measured?  Are they pixels, are they columns, something else?

Couldn't they be markers ? I think it's the easiest to work with, in the
case the text changes.

-- 
Nico

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

> What are BEG1, END1, BEG2, END2, etc. values here?

Buffer positions.  These are needed for the new non-contiguous notion of
region introduced by rectangles (and which could be used for other
notions of selections, e.g. the recently mentioned "inverse region").


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

>> One way to handle it generically, is to change call-interactively such
>> that when the argument is "r", the command is called once for each
>> "extent".  Obviously, this won't work correctly for all cases, tho.
> Also this won't work for commands that use region-beginning/region-end
> in the interactive spec (like query-replace for bug#13178/bug#19829).

Of course, I don't think we can handle those commands without
manual changes.

> If the command used region-beginning and region-end, this means
> changing them to e.g. plural region-beginnings and region-ends.

Here again, I'd rather use a list of pairs than a pair of lists, so we
wouldn't want to have 2 functions (one to get the list of beginnings
and another to get the list of ends).


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

> What are BEG1, END1, BEG2, END2, etc. values here?

They should have the same type as the return values of
region-beginning/region-end and the interactive space "r"
which are of the marker type.  So the patch in bug#19829 should be
improved by another patch (both will be installed after finishing
designing this feature):

diff --git a/lisp/emulation/cua-rect.el b/lisp/emulation/cua-rect.el
index 044939f..a314d33 100644
--- a/lisp/emulation/cua-rect.el
+++ b/lisp/emulation/cua-rect.el
@@ -676,9 +676,9 @@ (defun cua--extract-rectangle-positions ()
         (lambda (s e l r _v)
            (goto-char s)
            (move-to-column l)
-           (setq s (point))
+           (setq s (point-marker))
            (move-to-column r)
-           (setq e (point))
+           (setq e (point-marker))
            (setq rect (cons (cons s e) rect)))))
     (nreverse rect)))
 
diff --git a/lisp/rect.el b/lisp/rect.el
index 271f720..7a2c5a3 100644
--- a/lisp/rect.el
+++ b/lisp/rect.el
@@ -265,7 +265,8 @@ (defun extract-rectangle-positions (start end)
     (apply-on-rectangle
      (lambda (startcol endcol)
        (move-to-column startcol)
-       (push (cons (prog1 (point) (move-to-column endcol)) (point))
+       (push (cons (prog1 (point-marker) (move-to-column endcol))
+		   (point-marker))
 	     positions))
      start end)
     (nreverse positions)))

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

>> If the command used region-beginning and region-end, this means
>> changing them to e.g. plural region-beginnings and region-ends.
>
> Here again, I'd rather use a list of pairs than a pair of lists, so we
> wouldn't want to have 2 functions (one to get the list of beginnings
> and another to get the list of ends).

A list of pairs would be much cleaner, indeed.  The problem is that
there are 2 arguments already in region-handling commands: BEG and END.
Using one of them for a list of pairs, and leaving another unused
is not a nice thing to do.

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

> -           (setq s (point))
> +           (setq s (point-marker))

Why return markers rather than integers?
`region-beginning/region-end' return integers, not markers.
The function receiving these positions can easily turn them into markers
if it needs it.


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

> A list of pairs would be much cleaner, indeed.  The problem is that
> there are 2 arguments already in region-handling commands: BEG and END.
> Using one of them for a list of pairs, and leaving another unused
> is not a nice thing to do.

I think this "lack of niceness" is not strong enough to override the
advantage of using a list of pairs.


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 16 Mar 2015 22:58:56 +0200
> Cc: Pål Grønås Drange
> 	<paal.drange@gmail.com>, 20070@debbugs.gnu.org
> 
> A list of pairs would be much cleaner, indeed.  The problem is that
> there are 2 arguments already in region-handling commands: BEG and END.
> Using one of them for a list of pairs, and leaving another unused
> is not a nice thing to do.

Why not?  AFAIK, we already have APIs where an argument could be
either a buffer position or a string.

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

>> A list of pairs would be much cleaner, indeed.  The problem is that
>> there are 2 arguments already in region-handling commands: BEG and END.
>> Using one of them for a list of pairs, and leaving another unused
>> is not a nice thing to do.
>
> I think this "lack of niceness" is not strong enough to override the
> advantage of using a list of pairs.

I still have doubts about the API, but anyway here is the implementation
that (ab)uses the first arg BEG:

diff --git a/src/casefiddle.c b/src/casefiddle.c
index 8755353..b77a0e6 100644
--- a/src/casefiddle.c
+++ b/src/casefiddle.c
@@ -306,14 +306,25 @@
   return Qnil;
 }
 
-DEFUN ("downcase-region", Fdowncase_region, Sdowncase_region, 2, 2, "r",
+DEFUN ("downcase-region", Fdowncase_region, Sdowncase_region, 2, 2,
+       "(list (funcall region-extract-function 'positions) nil)",
        doc: /* Convert the region to lower case.  In programs, wants two arguments.
 These arguments specify the starting and ending character numbers of
 the region to operate on.  When used as a command, the text between
 point and the mark is operated on.  */)
   (Lisp_Object beg, Lisp_Object end)
 {
-  casify_region (CASE_DOWN, beg, end);
+  if (CONSP (beg))
+    {
+      while (CONSP (beg))
+	{
+	  casify_region (CASE_DOWN, XCAR (XCAR (beg)), XCDR (XCAR (beg)));
+	  beg = XCDR (beg);
+	}
+    }
+  else
+    casify_region (CASE_DOWN, beg, end);
+
   return Qnil;
 }
 

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Stefan Monnier]

> I still have doubts about the API, but anyway here is the implementation
> that (ab)uses the first arg BEG:

That looks OK, thanks.

I'm tempted to say that "r" should do (funcall region-extract-function
'positions), and fallback to passing the result as 2 separate positions
if the result is a singleton list.  Also, if we change "r" then I think
it might make sense to pass a special value in the second arg to be more
explicit then just nil (e.g. pass :split-region).


        Stefan

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

> I'm tempted to say that "r" should do (funcall region-extract-function
> 'positions), and fallback to passing the result as 2 separate positions
> if the result is a singleton list.

Then all unfixed commands will fail on the rectangular region,
but they do the wrong thing now anyway, so this is not a problem.

>                                     Also, if we change "r" then I think
> it might make sense to pass a special value in the second arg to be more
> explicit then just nil (e.g. pass :split-region).

Then the command signature could be changed from

  (downcase-region BEG END)

to e.g.

  (downcase-region REGION REGION-TYPE)

or maybe to signify their dual nature

  (downcase-region REGION-OR-BEG REGION-TYPE-OR-END)

and I see no better way to do this.

bug#20070: 24.3; have commands such as downcase-region act on a rectangle region

[Juri Linkov]

forcemerge 19829 20070
thanks

> While this is not the same bug as bug#19829, it's fundamentally the same
> issue, and they should be fixed in the same way.

So I merged it with bug#19829 that provides a common patch.