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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 31 May 2025 12:18:09 +0300

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.