From: Ruijie Yu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Rah Guzar <rahguzar@zohomail.eu>
Cc: 61602@debbugs.gnu.org
Subject: bug#61602: [PATCH]: comint-mode redirection
Date: Fri, 12 May 2023 10:16:48 +0800 [thread overview]
Message-ID: <sdv5y8y8fan.fsf@netyu.xyz> (raw)
In-Reply-To: <87pm763gfy.fsf@zohomail.eu>
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). Take a look at its manpage and ask if
you have any questions. 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.
And there are guidelines on commit messages, see /CONTRIBUTE on
emacs.git.
Further in-line comments below.
Rah Guzar via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes:
> Dear Emacs Maintainers,
> A while back I sent a patch that addresses the points in this bug report. I
> have since received the confirmation that I have completed the copyright
> paperwork, so I am bringing it to your attention again. This is my first time
> contributing so please let me know if I should do somethings differently or if
> changes are needed.
>
> Thanks,
> Rah Guzar
>
> [2. text/x-patch; comint-redirect.patch]...
> diff --git a/lisp/comint.el b/lisp/comint.el
> index 682b555a33c..98f4d315d64 100644
> --- 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.
> Good choices:
> Canonical Lisp: \"^[^> \\n]*>+:? *\" (Lucid, franz, kcl, T, cscheme, oaklisp)
> @@ -3637,7 +3640,12 @@ comint-redirect-output-buffer
> (defvar comint-redirect-finished-regexp nil
> "Regular expression that determines when to stop redirection in Comint.
> When the redirection filter function is given output that matches this regexp,
> -the output is inserted as usual, and redirection is completed.")
> +the output is inserted as usual, and redirection is completed.
> +This is an internal variable set by `comint-redirect-setup' and setting it
> +directly has no effect.")
If this is indeed a private variable, why does it contain no
double-dashes in its name prior to your changes?
Also, here and elsewhere, except for the first line, there should
generally be one empty line between paragraphs.
> +
> +(defvar comint-redirect-hook nil
> + "Hook run when a redirection finishes.")
Does it make sense for a user to customize the hook? If so, you should
convert this variable into a `defcustom'.
> (defvar comint-redirect-insert-matching-regexp nil
> "If non-nil, the text that ends a redirection is included in it.
> @@ -3833,11 +3841,13 @@ comint-redirect-send-command
>
> ;;;###autoload
> (defun comint-redirect-send-command-to-process
> - (command output-buffer process echo &optional no-display)
> + (command output-buffer process echo &optional no-display finished-regexp)
> "Send COMMAND to PROCESS, with output to OUTPUT-BUFFER.
> With prefix arg, echo output in process buffer.
>
> -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.
If you are referring to the change below from `cominit-prompt-regexp' to
`(or finished-regexp comint-prompt-regexp)', then the current form is
ambiguous, and maybe you should say something like this:
If F-R is non-nil, it is used as `c-r-f-r'. Otherwise `c-p-r' is
used as `c-r-f-r'.
> (interactive "sCommand: \nBOutput Buffer: \nbProcess Buffer: \nP")
> (let* (;; The process buffer
> (process-buffer (if (processp process)
> @@ -3858,7 +3868,7 @@ comint-redirect-send-command-to-process
> (comint-redirect-setup
> output-buffer
> (current-buffer) ; Comint Buffer
> - comint-prompt-regexp ; Finished Regexp
> + (or finished-regexp comint-prompt-regexp) ; Finished Regexp
> echo) ; Echo input
>
> ;; Set the filter.
--
Best,
RY
next prev parent reply other threads:[~2023-05-12 2:16 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-18 11:26 bug#61602: 29.0.60; comint-mode redirection Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-04-09 10:04 ` Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-11 17:45 ` bug#61602: [PATCH]: " Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-11 18:14 ` Eli Zaretskii
2023-05-11 18:35 ` Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-12 5:28 ` Eli Zaretskii
2023-05-12 2:16 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2023-05-12 6:42 ` Eli Zaretskii
2023-05-12 6:58 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-12 7:54 ` Andreas Schwab
2023-05-12 7:25 ` Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-12 7:11 ` Rah Guzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=sdv5y8y8fan.fsf@netyu.xyz \
--to=bug-gnu-emacs@gnu.org \
--cc=61602@debbugs.gnu.org \
--cc=rahguzar@zohomail.eu \
--cc=ruijie@netyu.xyz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).