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

[Eli Zaretskii]

Ping! Ping!  Any interest in pursuing this issue further?  If so,
could you please answer my questions below?

> Cc: 69914@debbugs.gnu.org
> Date: Sat, 06 Apr 2024 11:58:51 +0300
> From: 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?
> > 
> > 
> > 
> > 
> 
> 
> 
>