bug#20626: Wishlist: M-x shell-command-on-rectangle-region

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[asparagus]

Wishlist:
M-x shell-command-on-rectangle-region

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

> Wishlist:
> M-x shell-command-on-rectangle-region

As you can see in bug#20070, the effort to make commands rectangleable
had stalled some time ago due to the need to decide how to handle
backward-compatibility of the existing region arguments, e.g. in

  (shell-command-on-region START END COMMAND &optional OUTPUT-BUFFER REPLACE
                           ERROR-BUFFER DISPLAY-ERROR-BUFFER)

how to send the boundaries of the rectangular region in START and END.

One idea is to handle it like recently we handled backward-compatibility
for saving dired positions in saveplace.el where we used a new format like

  ("~" (dired-filename . "~/.emacs.d/places"))

Using something like this means sending the rectangular bounds
either in START or END in the new format like

  (rect (1 . 2) (3 . 4))

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

>> Wishlist:
>> M-x shell-command-on-rectangle-region
>
> As you can see in bug#20070, the effort to make commands rectangleable
> had stalled some time ago due to the need to decide how to handle
> backward-compatibility of the existing region arguments, e.g. in
>
>   (shell-command-on-region START END COMMAND &optional OUTPUT-BUFFER REPLACE
>                            ERROR-BUFFER DISPLAY-ERROR-BUFFER)
>
> how to send the boundaries of the rectangular region in START and END.
>
> One idea is to handle it like recently we handled backward-compatibility
> for saving dired positions in saveplace.el where we used a new format like
>
>   ("~" (dired-filename . "~/.emacs.d/places"))
>
> Using something like this means sending the rectangular bounds
> either in START or END in the new format like
>
>   (rect (1 . 2) (3 . 4))

Sorry, I was wrong.  I realized now that query-replace has quite
a different requirement.  query-replace needs rectangular boundaries
to limit the search for replacements, whereas shell-command-on-region
should extract the rectangular region as strings and replace it with
the result of the command.  Here is a working prototype that demonstrates
its possible implementation:

(define-advice shell-command-on-region
    (:around (orig-fun start end command
                       &optional output-buffer replace
                       error-buffer display-error-buffer))
  (if (and (boundp 'rectangle-mark-mode) rectangle-mark-mode)
      (let ((input (mapconcat 'identity (delete-extract-rectangle start end) "\n"))
            output)
        (with-temp-buffer
          (insert input)
          (call-process-region (point-min) (point-max)
                               shell-file-name t t
                               nil shell-command-switch
                               command)
          (setq output (split-string (buffer-string) "\n")))
        (goto-char start)
        (insert-rectangle output))
    (funcall orig-fun start end command
             output-buffer replace
             error-buffer display-error-buffer)))

This is another case to take into account when designing the interface,
i.e. in this case the list of boundaries in the arg START is not necessary,
and I have no idea how to avoid `(if (and (boundp 'rectangle-mark-mode)
rectangle-mark-mode))'

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

> Sorry, I was wrong.  I realized now that query-replace has quite
> a different requirement.  query-replace needs rectangular boundaries
> to limit the search for replacements, whereas shell-command-on-region
> should extract the rectangular region as strings and replace it with
> the result of the command.  Here is a working prototype that demonstrates
> its possible implementation:
>
> (define-advice shell-command-on-region
>     (:around (orig-fun start end command
>                        &optional output-buffer replace
>                        error-buffer display-error-buffer))
>   (if (and (boundp 'rectangle-mark-mode) rectangle-mark-mode)
>       (let ((input (mapconcat 'identity (delete-extract-rectangle start end) "\n"))
>             output)
>         (with-temp-buffer
>           (insert input)
>           (call-process-region (point-min) (point-max)
>                                shell-file-name t t
>                                nil shell-command-switch
>                                command)
>           (setq output (split-string (buffer-string) "\n")))
>         (goto-char start)
>         (insert-rectangle output))
>     (funcall orig-fun start end command
>              output-buffer replace
>              error-buffer display-error-buffer)))
>
> This is another case to take into account when designing the interface,
> i.e. in this case the list of boundaries in the arg START is not necessary,
> and I have no idea how to avoid `(if (and (boundp 'rectangle-mark-mode)
> rectangle-mark-mode))'

For example, the command ‘kill-ring-save’ has the signature

  kill-ring-save (beg end &optional region)

with an additional boolean arg for the region.

And ‘rectangle--extract-region’ contains a condition
that checks for ‘rectangle-mark-mode’.

Combining these two prerequisites we could do a similar thing in
‘shell-command-on-region’ by adding a new arg and using it in the
function body:

diff --git a/lisp/simple.el b/lisp/simple.el
index 1868077..d022504 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3274,7 +3278,8 @@
 
 (defun shell-command-on-region (start end command
 				      &optional output-buffer replace
-				      error-buffer display-error-buffer)
+				      error-buffer display-error-buffer
+				      region)
   "Execute string COMMAND in inferior shell with region as input.
 Normally display output (if any) in temp buffer `*Shell Command Output*';
 Prefix arg means replace the region with it.  Return the exit code of
@@ -3337,7 +3342,8 @@ (defun shell-command-on-region (start end command
 		       current-prefix-arg
 		       current-prefix-arg
 		       shell-command-default-error-buffer
-		       t)))
+		       t
+		       rectangle-mark-mode)))
   (let ((error-file
 	 (if error-buffer
 	     (make-temp-file
@@ -3346,6 +3352,18 @@ (defun shell-command-on-region (start end command
 				    temporary-file-directory)))
 	   nil))
 	exit-status)
+    (if region
+        (let ((input (mapconcat 'identity (delete-extract-rectangle start end) "\n"))
+              output)
+          (with-temp-buffer
+            (insert input)
+            (call-process-region (point-min) (point-max)
+                                 shell-file-name t t
+                                 nil shell-command-switch
+                                 command)
+            (setq output (split-string (buffer-string) "\n")))
+          (goto-char start)
+          (insert-rectangle output))
     (if (or replace
 	    (and output-buffer
 		 (not (or (bufferp output-buffer) (stringp output-buffer)))))
@@ -3435,7 +3453,7 @@ (defun shell-command-on-region (start end command
 			      exit-status output))))
 	    ;; Don't kill: there might be useful info in the undo-log.
 	    ;; (kill-buffer buffer)
-	    ))))
+	    )))))
 
     (when (and error-file (file-exists-p error-file))
       (if (< 0 (nth 7 (file-attributes error-file)))

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Stefan Monnier]

>  (defun shell-command-on-region (start end command
>  				      &optional output-buffer replace
> -				      error-buffer display-error-buffer)
> +				      error-buffer display-error-buffer
> +				      region)
>    "Execute string COMMAND in inferior shell with region as input.
>  Normally display output (if any) in temp buffer `*Shell Command Output*';
>  Prefix arg means replace the region with it.  Return the exit code of
> @@ -3337,7 +3342,8 @@ (defun shell-command-on-region (start end command
>  		       current-prefix-arg
>  		       current-prefix-arg
>  		       shell-command-default-error-buffer
> -		       t)))
> +		       t
> +		       rectangle-mark-mode)))

Doesn't make sense: if the value determine the use of rectangles, the
arg shouldn't be called `region' but something like `rectangle'.

Notice how kill-ring-save takes a `region' argument and doesn't have any
rectangle-specific code.

I still believe that shell-command-on-region should not have
rectangle-specific code.  The current `region-extract-function' does let
you extract the region (rectangular or not) in order to pass it to
a shell command.  So you don't need any rectangle-specific code for that
part of shell-command-on-region.
OTOH There is indeed some functionality missing there to let you insert
the output in a rectangular way (whatever that means).


        Stefan

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

> I still believe that shell-command-on-region should not have
> rectangle-specific code.  The current `region-extract-function' does let
> you extract the region (rectangular or not) in order to pass it to
> a shell command.  So you don't need any rectangle-specific code for that
> part of shell-command-on-region.
> OTOH There is indeed some functionality missing there to let you insert
> the output in a rectangular way (whatever that means).

‘shell-command-on-region’ currently relies on the call to

   (call-process-region start end shell-file-name replace ...

but I'm not sure if ‘call-process-region’ is not too low level
to handle rectangular regions.  Otherwise, there should be a condition
in ‘shell-command-on-region’ to check for a rectangular region and
insert the output accordingly, so I see no way to avoid checking for
‘rectangle-mark-mode’ in ‘shell-command-on-region’.

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Stefan Monnier]

> in ‘shell-command-on-region’ to check for a rectangular region and
> insert the output accordingly, so I see no way to avoid checking for
> ‘rectangle-mark-mode’ in ‘shell-command-on-region’.

In any case checking specifically for a rectangular region is wrong.
Having a "fast-path" for the case of a simple contiguous region is fine,
but the "slow&complex" path is not just for rectangular regions:
the code should also work with a region made up of various
arbitrary chunks (there's currently no package that provides this
feature, but that's no excuse).


        Stefan

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

>> in ‘shell-command-on-region’ to check for a rectangular region and
>> insert the output accordingly, so I see no way to avoid checking for
>> ‘rectangle-mark-mode’ in ‘shell-command-on-region’.
>
> In any case checking specifically for a rectangular region is wrong.
> Having a "fast-path" for the case of a simple contiguous region is fine,
> but the "slow&complex" path is not just for rectangular regions:
> the code should also work with a region made up of various
> arbitrary chunks (there's currently no package that provides this
> feature, but that's no excuse).

Then what about adding a new arg REGION to ‘shell-command-on-region’
and other region-sensitive commands that will provide the region
configuration including information about region type and region shape,
e.g. a list in the form ‘(rectangle …)’.  Then the command's body
will check the car of the arg REGION and act accordingly depending
on the region shape.

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Stefan Monnier]

> e.g. a list in the form ‘(rectangle …)’.  Then the command's body
> will check the car of the arg REGION and act accordingly depending
> on the region shape.

Same difference.  It'd still have rectangle-specific code.

Instead it should call something like region-extract-function to do what
it needs to do and *this* thing will have rectangle-specific code (via
something like an add-function in rect.el).

IIUC the current region-extract-function doesn't satisfy all the needs
of shell-command-on-region, so either it will have to be extended again,
or we have to add more region-<foo>-function or equivalent methods.


        Stefan

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

> In any case checking specifically for a rectangular region is wrong.
> Having a "fast-path" for the case of a simple contiguous region is fine,
> but the "slow&complex" path is not just for rectangular regions:
> the code should also work with a region made up of various
> arbitrary chunks (there's currently no package that provides this
> feature, but that's no excuse).

It's more-less clear how to write code for the "slow&complex" path
using the existing ‘region-extract-function’ and a new function like
‘region-insert-function’ like I demonstrated with define-advice
for shell-command-on-region in the beginning of this thread (where
specific ‘insert-rectangle’ should be replaced with ‘region-insert-function’).

What is not clear is how to distinguish "slow&complex" from the "fast-path"?
Maybe with a new arg REGION?

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Stefan Monnier]

> What is not clear is how to distinguish "slow&complex" from the "fast-path"?
> Maybe with a new arg REGION?

I thought it would be pretty easy: if region-extract-function returns
a single contiguous chunk or multiple chunks.


        Stefan

bug#20626: Wishlist: M-x shell-command-on-rectangle-region

[Juri Linkov]

forcemerge 19829 20626
thanks

>> What is not clear is how to distinguish "slow&complex" from the "fast-path"?
>> Maybe with a new arg REGION?
>
> I thought it would be pretty easy: if region-extract-function returns
> a single contiguous chunk or multiple chunks.

‘query-replace’ will use the same design, so merged it with bug#19829
where I posted a composite patch.