fix: close remaining gaps found by the completeness audit
Addresses the partial ports listed in docs/rewrite/notes/review-completeness-codex.md (details in docs/rewrite/notes/gaps-X2.md): - Flatpak: the exit watcher no longer consumes the host command; a shared FlatpakSignalHandle lets closing a surface still SIGHUP the host process group. - Child environment is byte-preserving end to end (ghostty_os::EnvMap of OsStrings) and Snap paths are filtered like filterSnapPaths. - argv handling no longer panics on non-UTF-8 arguments. - URI percent-decoding keeps raw bytes; OSC 7 pwd is passed through without lossy conversion. - Keybinding comparison/hashing uses full Unicode case folding from a table generated from CaseFolding.txt (Unicode 17.0.0). - Link regexes set Oniguruma's whole-search retry limit through the new minimal ghostty-onig crate (one SAFETY-commented FFI call). - OSC 9 NUL-capacity check ordering matches Zig. - Wayland registry binds/clears globals advertised after startup. - Custom shaders may pass sampler2D to helper functions (naga adapter). - +list-themes preview renders OSC 8 links, styled underlines and follows color-scheme reports. Also includes the inspector's integration edits in shared files (renderer state hook, termio VT recording, core/apprt-gtk wiring), X1's config/input/cli additions for completion generation, and a fix for a race in the termio backpressure test (it wrote before `stty raw` applied, so the cooked line discipline dropped bytes past MAX_CANON). Test Plan: - cargo test --workspace - cargo clippy --workspace --all-targets -- -D warnings
This commit is contained in:
1 parent
90ad602969
commit
f4c020101c
61 files changed
+4210
-298
No files matched your search
@@ -11,6 +11,7 @@ ghostty-apprt.workspace = true
|
||||
ghostty-config.workspace = true
|
||||
ghostty-datastruct.workspace = true
|
||||
ghostty-font.workspace = true
|
||||
ghostty-inspector.workspace = true
|
||||
ghostty-input.workspace = true
|
||||
ghostty-os.workspace = true
|
||||
ghostty-renderer.workspace = true
|
||||
|
||||
@@ -17,6 +17,7 @@ use ghostty_input::binding::set::Value;
|
||||
use ghostty_input::binding::{Action as BindingAction, Flags, Scope, Set, Trigger, TriggerKey};
|
||||
use ghostty_input::key_encode;
|
||||
use ghostty_input::{Action as KeyAction, Key, KeyEvent, Mods};
|
||||
use ghostty_inspector as inspector;
|
||||
use ghostty_terminal::terminal::{MouseEvents, ScrollViewport, Terminal};
|
||||
use ghostty_termio::message::Message as IoMessage;
|
||||
use ghostty_vt::modes::Mode;
|
||||
@@ -331,10 +332,40 @@ impl Surface {
|
||||
// level.
|
||||
let event = Self::remap_event(&cfg, &event_orig);
|
||||
|
||||
// Setup our inspector event if we have an inspector.
|
||||
let mut insp_ev = self
|
||||
.inspector
|
||||
.borrow()
|
||||
.as_ref()
|
||||
.map(|_| inspector::KeyEvent::new(&event));
|
||||
|
||||
let result = self.key_callback_remapped(&cfg, &event, insp_ev.as_mut());
|
||||
|
||||
// When we're done processing, we always want to add the event to
|
||||
// the inspector. We have to check for the inspector again because
|
||||
// our keybinding might have closed it.
|
||||
if let Some(ev) = insp_ev
|
||||
&& let Some(insp) = self.inspector()
|
||||
{
|
||||
insp.lock().record_key_event(ev);
|
||||
self.queue_render();
|
||||
}
|
||||
|
||||
result
|
||||
}
|
||||
|
||||
/// The remainder of [`Surface::key_callback`] after the key remapping and
|
||||
/// the inspector event setup.
|
||||
fn key_callback_remapped(
|
||||
&self,
|
||||
cfg: &DerivedConfig,
|
||||
event: &KeyEvent<'_>,
|
||||
mut insp_ev: Option<&mut inspector::KeyEvent>,
|
||||
) -> Result<InputEffect, CoreError> {
|
||||
// Handle keybindings first. We need to handle this on all events
|
||||
// (press, repeat, release) because a press may perform a binding but
|
||||
// a release should not encode if we consumed the press.
|
||||
if let Some(v) = self.maybe_handle_binding(&cfg, &event) {
|
||||
if let Some(v) = self.maybe_handle_binding(cfg, event, insp_ev.as_deref_mut()) {
|
||||
return Ok(v);
|
||||
}
|
||||
|
||||
@@ -359,7 +390,7 @@ impl Surface {
|
||||
// highlight state.
|
||||
let mouse_mods = self.mouse.borrow().mods;
|
||||
if !mouse_mods.equal(event.mods) {
|
||||
self.refresh_mouse_for_mods_change(&event);
|
||||
self.refresh_mouse_for_mods_change(event);
|
||||
}
|
||||
|
||||
// Process the cursor state logic. This will update the cursor shape
|
||||
@@ -381,7 +412,7 @@ impl Surface {
|
||||
*pressed = 'event: {
|
||||
// We need to unset the allocated fields that will become
|
||||
// invalid
|
||||
let mut copy = OwnedKeyEvent::from_event(&event);
|
||||
let mut copy = OwnedKeyEvent::from_event(event);
|
||||
copy.utf8.clear();
|
||||
|
||||
// If we have a previous pressed key and we're releasing it
|
||||
@@ -411,7 +442,7 @@ impl Surface {
|
||||
|
||||
// Encode and send our key. If we didn't encode anything, then we
|
||||
// return the effect as ignored.
|
||||
let Some(write_req) = self.encode_key(&event) else {
|
||||
let Some(write_req) = self.encode_key(event, insp_ev) else {
|
||||
// No valid request means that we didn't encode anything.
|
||||
return Ok(InputEffect::Ignored);
|
||||
};
|
||||
@@ -491,6 +522,7 @@ impl Surface {
|
||||
&self,
|
||||
cfg: &DerivedConfig,
|
||||
event: &KeyEvent<'_>,
|
||||
mut insp_ev: Option<&mut inspector::KeyEvent>,
|
||||
) -> Option<InputEffect> {
|
||||
match event.action {
|
||||
// Release events never trigger a binding but we need to check
|
||||
@@ -586,7 +618,7 @@ impl Surface {
|
||||
// Store this event so that we can drain and encode on
|
||||
// invalid. We don't need to cap this because it is
|
||||
// naturally capped by the config validation.
|
||||
if let Some(req) = self.encode_key(event) {
|
||||
if let Some(req) = self.encode_key(event, insp_ev.as_deref_mut()) {
|
||||
self.keyboard.borrow_mut().sequence_queued.push(req);
|
||||
}
|
||||
|
||||
@@ -683,6 +715,9 @@ impl Surface {
|
||||
// Store our last trigger so we don't encode the release event
|
||||
self.keyboard.borrow_mut().last_trigger = Some(event.binding_hash());
|
||||
|
||||
if let Some(ev) = insp_ev {
|
||||
ev.binding = actions;
|
||||
}
|
||||
return Some(InputEffect::Consumed);
|
||||
}
|
||||
|
||||
@@ -869,7 +904,11 @@ impl Surface {
|
||||
|
||||
/// Encodes the key event into a write request. Returns `None` if
|
||||
/// nothing was encoded.
|
||||
fn encode_key(&self, event: &KeyEvent<'_>) -> Option<IoMessage> {
|
||||
fn encode_key(
|
||||
&self,
|
||||
event: &KeyEvent<'_>,
|
||||
insp_ev: Option<&mut inspector::KeyEvent>,
|
||||
) -> Option<IoMessage> {
|
||||
// Build our encoding options, which requires the lock.
|
||||
let opts = {
|
||||
let t = self.lock_terminal();
|
||||
@@ -887,6 +926,11 @@ impl Surface {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Copy the encoded data into the inspector event if we have one.
|
||||
if let Some(ev) = insp_ev {
|
||||
ev.pty.clone_from(&data);
|
||||
}
|
||||
|
||||
Some(IoMessage::write_req(&data))
|
||||
}
|
||||
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
pub mod binding;
|
||||
pub mod clipboard;
|
||||
pub mod config;
|
||||
pub mod inspector;
|
||||
pub mod keyboard;
|
||||
pub mod mouse;
|
||||
|
||||
@@ -251,6 +252,9 @@ pub struct Surface {
|
||||
/// Used to rate limit BEL handling.
|
||||
last_bell_time: Cell<Option<Instant>>,
|
||||
|
||||
/// The terminal inspector, if active (Zig: `inspector: ?*Inspector`).
|
||||
pub(crate) inspector: RefCell<Option<Arc<ghostty_inspector::Shared>>>,
|
||||
|
||||
/// Debug screenshot state (see `debug_screenshot`).
|
||||
created: Instant,
|
||||
debug_shot_done: Cell<bool>,
|
||||
@@ -474,6 +478,7 @@ impl Surface {
|
||||
command_timer: Cell::new(None),
|
||||
search: RefCell::new(None),
|
||||
last_bell_time: Cell::new(None),
|
||||
inspector: RefCell::new(None),
|
||||
created: Instant::now(),
|
||||
debug_shot_done: Cell::new(false),
|
||||
});
|
||||
@@ -1798,7 +1803,7 @@ impl Drop for Surface {
|
||||
/// (`rt_surface.defaultTermioEnv()`, which already removed the variables it
|
||||
/// does not want to leak); only if it provides nothing (the Zig error path)
|
||||
/// do we fall back to the process environment (`getEnvMap`).
|
||||
fn rt_surface_env(rt: &dyn RtSurface) -> std::collections::HashMap<String, String> {
|
||||
fn rt_surface_env(rt: &dyn RtSurface) -> ghostty_os::EnvMap {
|
||||
let rt_env = rt.default_termio_env();
|
||||
if rt_env.is_empty() {
|
||||
return ghostty_os::env::get_env_map();
|
||||
@@ -1960,4 +1965,16 @@ impl CoreSurface for Surface {
|
||||
ydpi: s.ydpi,
|
||||
}
|
||||
}
|
||||
|
||||
fn activate_inspector(&self) -> Result<(), CoreError> {
|
||||
Surface::activate_inspector(self)
|
||||
}
|
||||
|
||||
fn deactivate_inspector(&self) {
|
||||
Surface::deactivate_inspector(self);
|
||||
}
|
||||
|
||||
fn render_inspector(&self, ui: &ghostty_inspector::imgui::Ui) {
|
||||
Surface::render_inspector(self, ui);
|
||||
}
|
||||
}
|
||||
@@ -975,6 +975,13 @@ impl Surface {
|
||||
) -> Result<bool, CoreError> {
|
||||
let cfg = self.cfg();
|
||||
|
||||
// If we have an inspector, we always queue a render
|
||||
let _inspector_render = self
|
||||
.inspector
|
||||
.borrow()
|
||||
.is_some()
|
||||
.then(|| RenderOnDrop(self));
|
||||
|
||||
// Always record our latest mouse state
|
||||
let hidden = {
|
||||
let mut m = self.mouse.borrow_mut();
|
||||
@@ -1915,6 +1922,10 @@ impl Surface {
|
||||
// The mouse position in the viewport
|
||||
let pos_vp = self.pos_to_viewport(f64::from(pos.x), f64::from(pos.y));
|
||||
|
||||
// If we have an inspector, we need to always record position
|
||||
// information
|
||||
self.inspector_record_cursor_pos(f64::from(pos.x), f64::from(pos.y));
|
||||
|
||||
// We always reset the over link status because it will be
|
||||
// reprocessed below. But we need the old value to know if we need to
|
||||
// undo mouse shape changes.
|
||||
@@ -2217,6 +2228,15 @@ fn button_from_index(i: usize) -> Option<MouseButton> {
|
||||
})
|
||||
}
|
||||
|
||||
/// Queues a render when dropped (Zig: `defer self.queueRender()`).
|
||||
struct RenderOnDrop<'a>(&'a Surface);
|
||||
|
||||
impl Drop for RenderOnDrop<'_> {
|
||||
fn drop(&mut self) {
|
||||
self.0.queue_render();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -143,12 +143,12 @@ impl RtSurface for FakeRt {
|
||||
}
|
||||
}
|
||||
}
|
||||
fn default_termio_env(&self) -> Vec<(String, String)> {
|
||||
fn default_termio_env(&self) -> Vec<(std::ffi::OsString, std::ffi::OsString)> {
|
||||
// Like GTK: the process environment minus what the runtime does not
|
||||
// want to leak (here HOME stands in for GDK_DISABLE & co), plus extras.
|
||||
let mut env: Vec<(String, String)> =
|
||||
std::env::vars().filter(|(k, _)| k != "HOME").collect();
|
||||
env.push(("TERM_PROGRAM_TEST".to_owned(), "1".to_owned()));
|
||||
let mut env: Vec<(std::ffi::OsString, std::ffi::OsString)> =
|
||||
std::env::vars_os().filter(|(k, _)| k != "HOME").collect();
|
||||
env.push(("TERM_PROGRAM_TEST".into(), "1".into()));
|
||||
env
|
||||
}
|
||||
fn gl_proc_address(&self, name: &str) -> *const std::ffi::c_void {
|
||||
|
||||
Reference in new issue
Block a user