bug#61602: [PATCH]: comint-mode redirection

[Ruijie Yu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 61602@debbugs.gnu.org
>> Date: Fri, 12 May 2023 10:16:48 +0800
>> From:  Ruijie Yu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> 
>> Thanks for the patch.  First of all, when sending a patch(set) for
>> Emacs, you need to run something like this:
>> 
>>     $ git format-patch
>> 
>> and send the generated file(s).
>
> No, this is not a requirement around here.  We prefer format-patch,
> indeed, but we don't enforce it.  Simple diffs are perfectly
> acceptable.

Yes, I should have worded that way.  I meant exactly what you said: it
is a preferred way but not a requirement.

>> What you have sent is a "diff" file, which bears no commit messages.
>> At least in Emacs contributions, patches should usually come
>> together with their commit messages.
>
> That is true.  But adding commit log messages doesn't have to be via
> "git format-patch".

Yes.

>> > --- a/lisp/comint.el
>> > +++ b/lisp/comint.el
>> > @@ -161,7 +161,10 @@ comint-prompt-regexp
>> >  Defaults to \"^\", the null string at BOL.
>> >  
>> >  This variable is only used if the variable
>> > -`comint-use-prompt-regexp' is non-nil.
>> > +`comint-use-prompt-regexp' is non-nil.  The exception to
>> > +this is redirection.  Many commands including
>> > +`comint-redirect-send-command-to-process' use it as
>> > +`comint-redirect-finished-regexp'.
>> 
>> This paragraph sounds a bit weird, but I don't know how to reword it.
>> Maybe someone else can help.
>
> I'm not sure the addition is necessary in the first place.  What is
> its purpose? why is it useful to mention those exceptional cases?
>
>> Also, here and elsewhere, except for the first line, there should
>> generally be one empty line between paragraphs.
>
> This is not a requirement.  It's a judgment call whether an empty line
> is needed or not.

Noted.

>> > -If NO-DISPLAY is non-nil, do not show the output buffer."
>> > +If NO-DISPLAY is non-nil, do not show the output buffer.
>> > +If FINISHED-REGEXP is non-nil it is used as `comint-redirect-finished-regexp'
>> > +instead of `comint-prompt-regexp'."
>> 
>> Please clarify what "it" is.
>
> I think it's clear in this case: there's no other candidate for being
> "it" here except FINISHED-REGEXP.

When I first read this sentence, I interpretted it as:

    If F-R is non-nil, F-R is used as `c-r-f-r'.  Otherwise F-R is used
    as `c-p-r'.

I don't know.  Maybe you see it differently, in which case I'm fine with
not changing it.

-- 
Best,


RY