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

From: Stéphane Zermatten <[email-masked]>
Date: Tue, 13 May 2025 17:24:29 +0300

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. 

￼