Re: [BUG] comint-strip-ctrl-m doesn't function as documentation states

[Jonathan <public@jds.work>]

[-- Attachment #1: Type: text/plain, Size: 4424 bytes --]

Apologies for the misleading subject. I had assumed that discussion of an approach about that particular filter function would happen here as technically it’s not a bug but inaccurate behavior by way of the documentation. I’m unsure if someone knows whether or not it’s “inaccuracy” is intentional and whether there’s a preferred approach to fixing.

If this isn’t the place for that discussion I can post in the bug-gnu-emacs mailing list.

- Jonathan

On Tue, Mar 19, 2024 at 10:29 PM, Po Lu <[luangruo@yahoo.com](mailto:On Tue, Mar 19, 2024 at 10:29 PM, Po Lu <<a href=)> wrote:

> Jonathan <public@jds.work> writes:
>
>> Hey folks,
>>
>> 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))=.
>>
>> #+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 brefity
>> )))
>> #+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.
>>
>> 1. Update the documentation and leave as is. This is the simplest
>> solution and would just require doc-string updates to indicate that
>> =comint-strip-ctrl-m= is a "unique" filter function among the other
>> filter functions that exist. This does not seem preferable to me.
>>
>> 2. Update the implementation of =comint-strip-ctrl-m= itself to
>> conform it to the documented API. This would mean anything currently
>> depending on it reading the =current-buffer= would break, and since
>> there are plenty of unknowns in that regard, this also does not seem
>> preferable.
>>
>> 3. Add a new version of the function with a different name that
>> conforms to the documented API =comint-strip-ctrl-m-output= or
>> something similar and deprecate the original.
>>
>> If we do decide to deprecate the original, I'm happy to include a
>> deprecation warning and keep an eye on it popping up in core to ensure
>> that we handle those issues over time.
>>
>> Any guidance would be useful. Thank you all for you're hard work.
>>
>> - Jonathan
>>
>> PS: Additional Context as promised:
>>
>> I was developing a package that runs SQL queries in a "hidden" SQLi
>> buffer and so I needed to strip carriage return characters out of the
>> output. Using this filter I had thought it would perform the task, but
>> it did not. So digging through the documentation I discovered this
>> error. I think it's pretty reasonable that filter functions conform to
>> the documented api or should at least be noted otherwise.
>
> Thanks. Please direct bug reports to bug-gnu-emacs@gnu.org that the bug
> tracker may record its progress: this list is for discussion that
> directly concerns development only.

[-- Attachment #2: Type: text/html, Size: 5239 bytes --]