bug#30280: async-shell-command-display-buffer doesn't work anymore

[Tino Calancha <tino.calancha@gmail.com>]

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


Thank you Basil for your prompt response!

Your patches LGTM.
Let's wait few more days if some people want to make further comments.
Otherwise, I will push them in your name into the master branch.

On Wed, 9 May 2018, Basil L. Contovounesios wrote:

>> I)
>>> +(declare-function comint-output-filter "comint" (process string))
>>> +
>> What is the purpose of this? AFICT no warning is shown when compiling
>> the file.
>
> On my end, removing the function declaration and invoking 'make' results
> in the following output:
>
> ...
> make[2]: Entering directory '/home/blc/.local/src/emacs/lisp'
>  ELC      ../lisp/simple.elc
> Reloading stale loaddefs.el
> Loading /home/blc/.local/src/emacs/lisp/loaddefs.el (source)...
>
> In end of data:
> simple.el:9030:1:Warning: the function ‘comint-output-filter’ is not known to
>    be defined.
> ...
>
> Note that this warning is only emitted when comint-output-filter is
> #'-quoted.  This is in line with the usual behaviour of the
> byte-compiler that I am accustomed to, i.e. I don't see anything out of
> the ordinary here.
Oh, I see.  I overlooked the fact that we changed `quote' with `function'.
With:
make bootstrap
there is no warning.
I am OK with clearing the warning in all cases.


>> * We require `shell.el' inside `shell-coomand'.
>> * `shell.el' requires `comint.el'.
>
> Yes, I understand that using comint-output-filter at this point in the
> program is kosher, but the byte-compiler evidently does not.
>
>> Is the purpose to serve as documentation? In that case I don't think we
>> need it (the prefix 'comint-' already makes obvious where this function
>> belongs to).
>
> No, the only intention is to pacify the byte-compiler.
>
>> II)
>> It's better to keep consistent with the indentation of the function you
>> are modifying:  here, `shell-command' is indenting with TAB.
>>
>> Tip:
>> You can see the tabs searching them with:
>> C-s C-q C-I
>> or you can persistenly highlight them with:
>> M-s h r C-I RET RET
>
> Thanks for the tip.  I personally prefer to use [global-]whitespace-mode
> with a whitespace-style setting which includes (face tab-mark).
> I additionally avoid accidentally committing tabs by configuring the git
> option core.whitespace to include tab-in-indent.
>
>> For instance, here you are changing:
>> 1) ' ---> #'
>> ;; and
>> 2) \t\t\s\s 000> \s\s\s\s...\s (18 white spaces)
>>
>> Please, do not change 2).
>
> I have no strong feeling on this; I was merely going along with the
> (emacs-lisp-mode . ((indent-tabs-mode . nil))) setting in the project's
> toplevel dir-locals-file, as well what I had inferred to be accepted
> policy (as Noam mentions in a separate email) from following Emacs
> development for the last couple of years.
>
> If it weren't for the above and the fact that most everything I have
> come across in the Emacs tree has Frankindentation (including the target
> function shell-command), I would be more inclined to remain consistent
> with the surrounding source.
>
> Let me know if it's still a problem and I'll gladly resend the patches
> with indent-tabs-mode enabled.
It's OK, Frankenstein is a glorious novel after all; I recommend anyone to 
revisit such classical masterpiece :-)