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

[Eli Zaretskii <eliz@gnu.org>]

Ping! Could you please answer my questions below?

> Cc: 69914@debbugs.gnu.org
> Date: Sat, 23 Mar 2024 09:30:16 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Wed, 20 Mar 2024 14:15:39 +0000
> > From:  Jonathan via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> > 
> > 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?
> 
> 
> 
>