Subject: bug#69914: comint-strip-ctrl-m doesn't function as documentation states

From: Eli Zaretskii <[email-masked]>
Date: Sat, 23 Mar 2024 09:30:16 +0200

> Date: Wed, 20 Mar 2024 14:15:39 +0000
> From:  Jonathan via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> There appears to either be a bug or just inaccurate documentation of =comint-strip-ctrl-m=. At the very bottom, I've included some context about my use case by which I discovered this bug that may or may not be relevant to you. The documentation for that function states:
> 
> #+begin_quote
> Strip trailing ^M characters from the current output group.
> 
> This function could be on comint-output-filter-functions or bound to a key.
> #+end_quote
> 
> =comint-output-filter-functions= states the following:
> 
> #+begin_quote
> ...These functions get one argument, a string containing the text as originally
> inserted.  Note that this might not be the same as the buffer contents between
> comint-last-output-start and the buffer's process-mark, if other filter
> functions have already modified the buffer.
> #+end_quote
> 
> Looking at the implementation of =comint-strip-ctrl-m= it appears that it completely ignores the =string= argument and instead uses =(get-buffer-process (current-buffer))= in direct contradiction to the documentation.

Actually, AFAICT, almost all of the filtering functions intended for
comint-output-filter-functions ignore its string argument.  Isn't that
so?

> #+begin_src emacs-lisp
> (defun comint-strip-ctrl-m (&optional _string interactive)
>   "Strip trailing `^M' characters from the current output group.
> This function could be on `comint-output-filter-functions' or bound to a key."
>   (interactive (list nil t))
>   (let ((process (get-buffer-process (current-buffer))))
>     (if (not process)
>         ;; This function may be used in
>         ;; `comint-output-filter-functions', and in that case, if
>         ;; there's no process, then we should do nothing.  If
>         ;; interactive, report an error.
>         (when interactive
>           (error "No process in the current buffer"))
>       ;;; rest omitted for brevity
>       )))
> #+end_src
> 
> This represents unexpected and undocumented behavior, as you anticipate =comint-strip-ctrl-m= to behave like any other comint output filter functions. I'd like to propose 3 different possible solutions for a patch and would like input on which is preferred as this code was originally introduced in 1994. I can submit a patch once a solution has been determined.

Given that almost all the filter functions behave the same (unless you
disagree), it sounds like ignoring the string is a de-facto standard
behavior.  So we should document that, and I guess adding a new
function, without deprecating the existing one, is the most reasonable
way ahead?