terminal: Retain relative order of responses (#16456)

Partially addresses #8497 (namely, the occurring with `delta`) As I mentioned in https://github.com/zed-industries/zed/issues/8497#issuecomment-2226896371, zed currently replies to OSC color requests (`OSC 10`, `OSC 11`, ...) out of order when immediately followed by another request (for example `CSI c`). All other terminals that [I have tested](https://github.com/bash/terminal-colorsaurus/blob/main/doc/terminal-survey.md) maintain relative order when replying to requests. ## Solution Respond to the `ColorRequest` in `process_event` (in the same place where other PTY writes happen) instead of queuing it up in the internal event queue. ## Alternative I initially thought that I could handle the color request similarly to the `TextAreaSizeRequest` where the size is stored in `last_content` and updated on `sync`. However this causes the terminal to report out-of-date values when a "set color" sequence is followed by a color request. ## Tests 1. `OSC 11; ?` (request bg color) + `CSI c` (request device attributes): ```shell printf '\e]11;?\e\\ \e[c' && cat -v # Expected result: ^[]11;rgb:dcdc/dcdc/dddd^[\^[[?6c # Current result: ^[[?6c^[]11;rgb:dcdc/dcdc/dddd^[\ (❌) # Result with this PR: ^[]11;rgb:dcdc/dcdc/dddd^[\^[[?6c (✅) # Result with alternative: ^[]11;rgb:dcdc/dcdc/dddd^[\^[[?6c (✅) ``` 2. `OSC 11; rgb:f0f0/f0f0/f0f0` (set bg color) + `OSC 11; ?` (request bg color) ```shell printf '\e]11;rgb:f0f0/f0f0/f0f0\e\\ \e]11;?\e\\' && cat -v # Expected result: ^[]11;rgb:f0f0/f0f0/f0f0^[\ # Current result: ^[]11;rgb:f0f0/f0f0/f0f0^[\ (✅) # Result with this PR: ^[]11;rgb:f0f0/f0f0/f0f0^[\ (✅) # Result with alternative: ^[]11;rgb:OUT_OF_DATE_COLOR_HERE^[\ (❌) ``` Release Notes: - N/A

crates/terminal/src/terminal.rs 🔗

@@ -18,7 +18,7 @@ use alacritty_terminal::{
         Config, RenderableCursor, TermMode,
     },
     tty::{self, setup_env},
-    vte::ansi::{ClearMode, Handler, NamedPrivateMode, PrivateMode, Rgb},
+    vte::ansi::{ClearMode, Handler, NamedPrivateMode, PrivateMode},
     Term,
 };
 use anyhow::{bail, Result};
@@ -127,7 +127,6 @@ pub enum MaybeNavigationTarget {
 
 #[derive(Clone)]
 enum InternalEvent {
-    ColorRequest(usize, Arc<dyn Fn(Rgb) -> String + Sync + Send + 'static>),
     Resize(TerminalSize),
     Clear,
     // FocusNextMatch,
@@ -688,9 +687,19 @@ impl Terminal {
                     cx.emit(Event::TitleChanged);
                 }
             }
-            AlacTermEvent::ColorRequest(idx, fun_ptr) => {
-                self.events
-                    .push_back(InternalEvent::ColorRequest(*idx, fun_ptr.clone()));
+            AlacTermEvent::ColorRequest(index, format) => {
+                // It's important that the color request is processed here to retain relative order
+                // with other PTY writes. Otherwise applications might witness out-of-order
+                // responses to requests. For example: An application sending `OSC 11 ; ? ST`
+                // (color request) followed by `CSI c` (request device attributes) would receive
+                // the response to `CSI c` first.
+                // Instead of locking, we could store the colors in `self.last_content`. But then
+                // we might respond with out of date value if a "set color" sequence is immediately
+                // followed by a color request sequence.
+                let color = self.term.lock().colors()[*index].unwrap_or_else(|| {
+                    to_alac_rgb(get_color_at_index(*index, cx.theme().as_ref()))
+                });
+                self.write_to_pty(format(color));
             }
             AlacTermEvent::ChildExit(error_code) => {
                 self.register_task_finished(Some(*error_code), cx);
@@ -714,12 +723,6 @@ impl Terminal {
         cx: &mut ModelContext<Self>,
     ) {
         match event {
-            InternalEvent::ColorRequest(index, format) => {
-                let color = term.colors()[*index].unwrap_or_else(|| {
-                    to_alac_rgb(get_color_at_index(*index, cx.theme().as_ref()))
-                });
-                self.write_to_pty(format(color))
-            }
             InternalEvent::Resize(mut new_size) => {
                 new_size.size.height = cmp::max(new_size.line_height, new_size.height());
                 new_size.size.width = cmp::max(new_size.cell_width, new_size.width());

terminal: Retain relative order of responses (#16456)

Change summary

Detailed changes

crates/terminal/src/terminal.rs 🔗