bug#22679: 25.0.91; ibuffer-do-shell-command-pipe truncate output

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

>> Thanks.  Now that I looked at the rest of the patch I see that you use
>> some kind of after/before hooks, so this approach doesn't really lend
>> itself to extracting into a more generally useful function which could
>> be used by shell-command (and others).
> Improving `shell-command' is out of the scope of my patch.  The goal is
> to avoid that these commands truncate the output: currently the output
> buffer just show the output for the last processed buffer.  The reason
> to that is that original implementation uses `shell-command', which used
> to erase the output buffer between two calls.

I understand.  I do not suggest to improve shell-command.
I suggest instead to extract from shell-command a new Elisp function
which includes the part of shell-command that you need, and then rewrite
shell-command by making it use the new function.

So shell-command would still work exactly as before, but its
implementation would now be spread over 2 functions, the inner one of
which would be useful to other Elisp libraries such as ibuffer.

>> Could you explain what made you use this approach, to see if there might
>> be some way to solve the problem while still making the code more
>> generally useful (and reduce redundancy rather than augment it)?
> I used the hooks (*) because at some point i thought that one user
> setting 'shell-command-not-erase-buffer' to a non-nil value, may
> expect that these ibuffer commands also set the point in the output
> buffer according with 'shell-command-not-erase-buffer'.

Hmm... I now see this new variable.  It has several problems indeed.
The "set the point" part is weird and I'm not sure it makes much sense
to fold it this way into the same var as the "don-t erase" part.

> If we decide that the behaviour of these commands should not depend
> on 'shell-command-not-erase-buffer', then a cleaner fix could be
> as follows:

I think we should first aim at a simple and clean fix, yes.

> -  (shell-command-on-region
> -   (point-min) (point-max) command))
> +  (let ((out-buf (get-buffer-create "*Shell Command Output*")))
> +    (with-current-buffer out-buf (goto-char (point-max)))
> +    (call-process-region (point-min) (point-max)
> +                         shell-file-name nil out-buf nil
> +                         shell-command-switch command)))

Here, for example, I'd expect maybe something like

    (new-shell-command-on-region
     (point-min) (point-max) command))

Maybe with one or two new additional args.  Or maybe

    (let ((out-buf (get-buffer-create shell-command-buffer-name)))
      (with-current-buffer out-buf (goto-char (point-max)))
      (call-shell-on-region (point-min) (point-max)
                            out-buf command)))


-- Stefan