Subject: bug#78263: [PATCH 1/3] Ignore OSC sequences in term instead of printing them

From: Jared Finder <[email-masked]>
Date: Sat, 31 May 2025 12:42:27 -0700

Sorry for being quiet. I was on work travel and am back now. I will take 
a look!

   -- MJF

On 2025-05-31 02:18, Eli Zaretskii wrote:
> Ping!  We are waiting for the copyright assignment, but meanwhile,
> Jared, could you please review the patch and comment?
> 
>> From: Stéphane Zermatten <[email-masked]>
>> Date: Tue, 13 May 2025 17:24:29 +0300
>> Cc: Jared Finder <[email-masked]>,
>>  Per Bothner <[email-masked]>,
>>  [email-masked]
>> 
>> Thank you for the review! I addressed the points you mentioned. See 
>> the updated patch attached to this e-mail.
>> 
>> > On 8 May 2025, at 13:03, Eli Zaretskii <[email-masked]> wrote:
>> >
>> >> From: Stéphane Zermatten <[email-masked]>
>> >> Date: Mon, 5 May 2025 19:57:45 +0300
>> >> Cc: [email-masked],
>> >> Dan Nicolaescu <[email-masked]>,
>> >> Eli Zaretskii <[email-masked]>
>> >>
>> >> I attached a merged version of all 3 patches to this e-mail that handles OSC sequences, so they’re handled right away.  I’m happy to do it that way, if it’s more convenient.
>> >
>> > Thanks, see some minor comments below.
>> >
>> > Jared, any further comments about the issue or the patch?
>> >
>> >> * lisp/term.el (term-emulate-terminal) handle OSC sequences
>> >
>> > The log message should mention all the functions/variables where you
>> > make changes, with a short description of each change.  Please also
>> > mention the bug number.
>> 
>> I updated the description, trying to follow the format of older 
>> commits.
>> 
>> >
>> >> +(defcustom term-osc-handlers '(("0" . ansi-osc-window-title-handler)
>> >> +                               ("2" . ansi-osc-window-title-handler)
>> >> +                               ("7" . ansi-osc-directory-tracker)
>> >> +                               ("8" . ansi-osc-hyperlink-handler))
>> >> +  "OSC sequence handler function alist.
>> >> +
>> >> +OSC (Operating System Command) is a category of ANSI escape sequence
>> >> +used in terminal application to introduce custom commands. Terminals
>> >> +ignore unknown OSC sequences by default. Handlers can be registered here
>> >> +to add support for new OSC sequences to `term'.
>> >> +
>> >> +Functions in this alist are passed matching valid OSC sequences as
>> >> +they're sent to the terminal.
>> >> +
>> >> +Valid OSC sequences are of the form
>> >> +  ESC ] code ; text BEL
>> >> +  ESC ] code ; text ESC \
>> >> +
>> >> +Each entry has the form (CODE . FUNCTION), where CODE is the string that
>> >> +appears before the semicolon.
>> >> +
>> >> +FUNCTION is called with two arguments CODE and TEXT with the term buffer
>> >> +active and its point and state active at the time the OSC sequence
>> >> +appeared in the stream."
>> >> +  :type '(alist :key-type string :value-type function)
>> >> +  :group 'term)
>> >
>> > New defcustoms should have a :version tag.
>> 
>> I added one. I guess we’re at 31.1, now.
>> 
>> >
>> >> +(defconst term-osc--max-bytes (* 32 1024 1024)
>> >> +  "Limit the length of OSC sequences to keep in memory.")
>> >
>> > This begs the question: what happens with longer sequences?  Can the
>> > doc string answer that question?
>> 
>> emulate-terminal goes back into its normal state, because presumably 
>> the end tag was lost.
>> I tried to describe that.
>> 
>> >
>> >> +                              (with-demoted-errors "term OSC error: %S"
>> >> +                                (funcall
>> >> +                                 func code
>> >> +                                 (decode-coding-string
>> >> +                                  (substring seq-str text-start end-mark)
>> >> +                                  locale-coding-system t)))))
>> >
>> > Why locale-coding-system?  Wouldn't the result of calling
>> > process-coding-system with the process running in term.el be a better
>> > candidate?
>> 
>> process-coding-system is just 'binary because emulate-terminal want to 
>> process the escape sequences before the string is decoded. Using 
>> locale-coding-system is the way strings are decoded everywhere in 
>> emulate-terminal.
>> 
>> >
>> > Last, but not least: you don't seem to have a copyright assignment on
>> > file, without which we cannot accept such a large contribution.  Would
>> > you like to start the paperwork of copyright assignment at this time?
>> > If yes, I will send you the form to fill and the instructions.
>> 
>> Yes, I still need to do that. I sent out a request to [email-masked] 
>> <mailto:[email-masked]> following the instructions in 
>> etc/copyright-assign.txt but have yet to hear back from them.