Fix crash and session saving.

This commit is contained in:
Greyson Parrelli committed 2026-09-24 12:43:41 -04:00
1 parent 2a0e03cc41
commit 0e8bc70151
9 files changed
+353 -25

No files matched your search

+5
View File
@@ -12,6 +12,7 @@ const glibunix = @import("glibunix");
const vt = @import("ghostty-vt"); const vt = @import("ghostty-vt");
const Pty = @import("Pty.zig"); const Pty = @import("Pty.zig");
const scope = @import("scope.zig");
const theme = @import("theme.zig"); const theme = @import("theme.zig");
const Session = @This(); const Session = @This();
@@ -208,6 +209,10 @@ pub fn create(
}); });
errdefer self.pty.deinit(); errdefer self.pty.deinit();
// Out of the app's own cgroup, so that the kernel killing something this
// shell runs is not also systemd's cue to kill the app. See `scope`.
scope.adopt(self.pty.pid);
if (opts.command.len > 0) { if (opts.command.len > 0) {
self.startup_command = try alloc.dupe(u8, opts.command); self.startup_command = try alloc.dupe(u8, opts.command);
} }
+15 -5
View File
@@ -23,7 +23,7 @@
//! //!
//! It lives in the *state* directory rather than beside `layouts.json` and //! It lives in the *state* directory rather than beside `layouts.json` and
//! `settings.json` in the config one. Those two are files a person writes; //! `settings.json` in the config one. Those two are files a person writes;
//! this one is written behind their back on every exit, and putting churn like //! this one is written behind their back every few seconds and on every exit, and putting churn like
//! that in a config directory — which plenty of people keep in version control //! that in a config directory — which plenty of people keep in version control
//! — would make every quit look like an edit. `$XDG_STATE_HOME` is where the //! — would make every quit look like an edit. `$XDG_STATE_HOME` is where the
//! spec puts exactly this: state that should carry across restarts but that //! spec puts exactly this: state that should carry across restarts but that
@@ -276,27 +276,37 @@ pub const SaveError = error{ OutOfMemory, WriteFailed };
/// nothing about that state worth offering to restore — so the offer should not /// nothing about that state worth offering to restore — so the offer should not
/// appear at the next launch, and the stale one it would otherwise still be /// appear at the next launch, and the stale one it would otherwise still be
/// holding should not either. /// holding should not either.
pub fn save(self: *Snapshot) SaveError!void { ///
/// `written` is a hash of what the caller last had written, and is updated to
/// this one's. A snapshot that would write the same bytes again is skipped: the
/// window is photographed every few seconds while it is open, and nearly every
/// one of those photographs is the same as the last.
pub fn save(self: *Snapshot, written: *u64) SaveError!void {
var path_buf: [max_path]u8 = undefined; var path_buf: [max_path]u8 = undefined;
const path = statePath(&path_buf) orelse return error.WriteFailed; const path = statePath(&path_buf) orelse return error.WriteFailed;
if (!self.any()) { if (!self.any()) {
remove(path); remove(path);
written.* = 0;
return; return;
} }
const text = try self.serialize();
defer self.arena.child_allocator.free(text);
const hash = std.hash.Wyhash.hash(0, text);
if (hash == written.*) return;
var dir_buf: [max_path]u8 = undefined; var dir_buf: [max_path]u8 = undefined;
const dir = stateDir(&dir_buf) orelse return error.WriteFailed; const dir = stateDir(&dir_buf) orelse return error.WriteFailed;
if (glib.mkdirWithParents(dir.ptr, 0o700) != 0) return error.WriteFailed; if (glib.mkdirWithParents(dir.ptr, 0o700) != 0) return error.WriteFailed;
const text = try self.serialize();
defer self.arena.child_allocator.free(text);
var err: ?*glib.Error = null; var err: ?*glib.Error = null;
if (glib.fileSetContents(path.ptr, text.ptr, @intCast(text.len), &err) == 0) { if (glib.fileSetContents(path.ptr, text.ptr, @intCast(text.len), &err) == 0) {
if (err) |e| e.free(); if (err) |e| e.free();
return error.WriteFailed; return error.WriteFailed;
} }
written.* = hash;
} }
/// Delete the file. Failures are ignored on purpose: the common one is it not /// Delete the file. Failures are ignored on purpose: the common one is it not
+79 -13
View File
@@ -10,6 +10,7 @@ const adw = @import("adw");
const gdk = @import("gdk"); const gdk = @import("gdk");
const gio = @import("gio"); const gio = @import("gio");
const glib = @import("glib"); const glib = @import("glib");
const glibunix = @import("glibunix");
const gobject = @import("gobject"); const gobject = @import("gobject");
const gtk = @import("gtk"); const gtk = @import("gtk");
const vt = @import("ghostty-vt"); const vt = @import("ghostty-vt");
@@ -152,6 +153,16 @@ restore_detail: *gtk.Label,
/// it — see `applyRestoreVisible`. /// it — see `applyRestoreVisible`.
restore_offered: bool = false, restore_offered: bool = false,
/// The timer that photographs the window while it is open, and the hash of the
/// photograph last written — see `onAutosave`.
autosave: c_uint = 0,
snapshot_written: u64 = 0,
/// The handlers that turn a polite kill into an ordinary quit — see
/// `onTerminateSignal`. Removed on teardown, which also hands each signal back
/// to its default action.
signal_sources: [terminate_signals.len]c_uint = @splat(0),
/// What a sidebar row is signaling. Defined with the dots themselves, since /// What a sidebar row is signaling. Defined with the dots themselves, since
/// a row and a pane header show the same five states for the same reasons. /// a row and a pane header show the same five states for the same reasons.
const Attention = Pane.Attention; const Attention = Pane.Attention;
@@ -495,6 +506,11 @@ pub fn create(alloc: std.mem.Allocator, app: *adw.Application) !*Window {
// otherwise withdraw it before it was ever made. // otherwise withdraw it before it was ever made.
self.offerRestore(); self.offerRestore();
self.autosave = glib.timeoutAddSeconds(autosave_interval_s, &onAutosave, self);
for (terminate_signals, &self.signal_sources) |signum, *source| {
source.* = glibunix.signalAdd(signum, &onTerminateSignal, self);
}
return self; return self;
} }
@@ -1383,8 +1399,9 @@ fn captureStartupTabs(ctx: ?*anyopaque) void {
// ------------------------------------------------------------------------- // -------------------------------------------------------------------------
// The session snapshot // The session snapshot
// //
// Every exit photographs the window — see `Snapshot` for what is in the // Every exit photographs the window, and so does a timer while it is open —
// photograph and what deliberately is not — and every launch that finds one // see `Snapshot` for what is in the photograph and what deliberately is not,
// and `onAutosave` for why once is not enough — and every launch that finds one
// offers it back from a banner at the foot of the sidebar. The offer is // offers it back from a banner at the foot of the sidebar. The offer is
// optional in both directions: dismissing it costs nothing, and taking it is a // optional in both directions: dismissing it costs nothing, and taking it is a
// single click rather than a dialog. // single click rather than a dialog.
@@ -1595,16 +1612,16 @@ fn buildSnapshotTab(self: *Window, entry: Snapshot.Tab) !*Tab {
/// Photograph the window for the next launch. /// Photograph the window for the next launch.
/// ///
/// Called from `onDestroy`, which is the one funnel every route out of the /// Called from `onAutosave` while the window is open, and from `onDestroy`,
/// window passes through — the quit confirmation being accepted, the last tab /// which is the one funnel every orderly route out of the window passes through
/// closing, the compositor closing the window — and which runs while the tabs /// — the quit confirmation being accepted, the last tab closing, the compositor
/// and their shells are all still alive. That last part is the reason it is /// closing the window, a SIGTERM — and which runs while the tabs and their
/// there and not in `quit`: a terminal's directory is read out of its live /// shells are all still alive. That last part is the reason it is there and not
/// child process, and after teardown there is nothing left to ask. /// in `quit`: a terminal's directory is read out of its live child process, and
/// after teardown there is nothing left to ask.
/// ///
/// Every failure below is a warning and a carry-on. This runs while the app is /// Every failure below is a warning and a carry-on. There is nowhere better to
/// already leaving, there is nowhere to report anything, and the worst case is /// report anything, and the worst case is one launch that has nothing to offer.
/// one launch that has nothing to offer.
fn saveSnapshot(self: *Window) void { fn saveSnapshot(self: *Window) void {
var snapshot: Snapshot = .init(self.alloc); var snapshot: Snapshot = .init(self.alloc);
defer snapshot.deinit(); defer snapshot.deinit();
@@ -1638,11 +1655,49 @@ fn saveSnapshot(self: *Window) void {
}; };
} }
snapshot.save() catch |err| { snapshot.save(&self.snapshot_written) catch |err| {
std.log.warn("could not write the session snapshot: {s}", .{@errorName(err)}); std.log.warn("could not write the session snapshot: {s}", .{@errorName(err)});
}; };
} }
/// How often the window is photographed while it is open.
///
/// An exit is not the only way out. The app can be killed outright — by
/// systemd tearing down the scope it runs in, by the OOM killer, by a crash —
/// and none of those reach `onDestroy`, so a snapshot taken only there is lost
/// along with the session it was meant to rescue. Photographing is cheap (a
/// `readlink` per terminal and a hash; the file is only written when something
/// actually moved), so this errs towards often.
const autosave_interval_s = 10;
fn onAutosave(data: ?*anyopaque) callconv(.c) c_int {
const self: *Window = @ptrCast(@alignCast(data.?));
// While the offer stands, the file on disk is the *last* session — the one
// the banner is offering — and the window is only showing the tabs it opened
// for itself. Overwriting it now would leave a crash before you answer with
// nothing to offer but those. Anything that changes the tab set withdraws the
// offer, so this resumes the moment the window holds work of its own.
if (!self.restore_offered) self.saveSnapshot();
return 1;
}
/// The signals that mean "please exit": systemd stopping the unit, the terminal
/// playpen was started from going away, and Ctrl+C in that terminal.
const terminate_signals = [_]c_int{ 15, 1, 2 }; // SIGTERM, SIGHUP, SIGINT
/// Quit as though the window had been closed, so the session is saved first.
///
/// Left to the default action, any of these kills the process where it stands
/// and `onDestroy` never runs. Destroying the window rather than closing it: a
/// signal is not a question, and the quit confirmation has nobody to ask.
fn onTerminateSignal(data: ?*anyopaque) callconv(.c) c_int {
const self: *Window = @ptrCast(@alignCast(data.?));
std.log.info("asked to exit by signal; saving the session and quitting", .{});
self.window.as(gtk.Window).destroy();
return 0;
}
/// Make `tab` the visible one. /// Make `tab` the visible one.
fn select(self: *Window, tab: *Tab) void { fn select(self: *Window, tab: *Tab) void {
self.updating = true; self.updating = true;
@@ -2351,8 +2406,19 @@ fn onDestroy(_: *adw.ApplicationWindow, self: *Window) callconv(.c) void {
if (self.closing) return; if (self.closing) return;
self.closing = true; self.closing = true;
// Both hold this window as their user data. Removing the signal handlers
// also puts the default actions back, so a second SIGTERM while teardown is
// stuck behind a shell that will not exit still gets rid of the process.
if (self.autosave != 0) _ = glib.Source.remove(self.autosave);
for (self.signal_sources) |source| {
if (source != 0) _ = glib.Source.remove(source);
}
// First, while every tab is still whole and every shell still running: the // First, while every tab is still whole and every shell still running: the
// directories in the snapshot are read out of live processes. // directories in the snapshot are read out of live processes. Written even
// if the last autosave took the same picture, since the exit is the one
// write that has to be sure it landed.
self.snapshot_written = 0;
self.saveSnapshot(); self.saveSnapshot();
// Before anything is freed: a scheme change arriving mid-teardown would // Before anything is freed: a scheme change arriving mid-teardown would
+119
View File
@@ -0,0 +1,119 @@
//! Each shell in its own systemd scope, so that the kernel's OOM killer taking
//! one process out of a tab cannot take the whole app with it.
//!
//! The failure this exists for: a desktop launcher starts playpen in a scope of
//! its own (niri's is `app-niri-playpen-<pid>.scope`), and by default every
//! process the app ever starts lands in that same scope — every shell, and
//! everything run from them. A scope's `OOMPolicy` defaults to `stop`, which
//! means that when the kernel OOM-kills *any* process in it, systemd responds by
//! stopping the unit, which is every other process in it. So a Gradle test
//! worker blowing through memory in one tab gets killed, as it should, and then
//! systemd kills playpen and every shell in every other tab for it. No crash,
//! no core dump, nothing in the app's own log: the journal just says the scope
//! "Failed with result 'oom-kill'".
//!
//! The policy cannot be changed on a scope that is already running — systemd
//! only takes `OOMPolicy` when the unit is created — so the app cannot fix the
//! scope it was launched into. What it can do is what GNOME's terminals do:
//! move each shell, right after it is forked, into a new transient scope of its
//! own with `OOMPolicy=continue`. The kernel still kills whatever it picked, and
//! that is the end of it; the shell that ran it, the other tabs and the window
//! all carry on. It also means that anything else in the system that acts on a
//! whole cgroup — `systemd-oomd`, `systemctl --user stop`, per-unit memory
//! accounting — sees one tab at a time rather than the whole app as one lump.
//!
//! Best-effort throughout. Outside a systemd user session the call fails, the
//! shell stays in the app's own cgroup, and everything else behaves exactly as
//! it did before this existed.
const std = @import("std");
const gio = @import("gio");
const glib = @import("glib");
const gobject = @import("gobject");
/// The session bus, fetched on first use. GIO hands every caller the same
/// shared connection, so this is only a cache of the lookup.
var bus: ?*gio.DBusConnection = null;
/// Whether a failure has been reported already. One line in the log says that
/// shells are not being isolated; one per tab would only repeat it.
var warned = false;
/// Move `pid` into a scope of its own. Returns immediately: the request goes out
/// asynchronously and the shell carries on starting while systemd moves it.
///
/// That leaves a window, between the fork and the move, in which anything the
/// shell starts stays behind in the app's cgroup. It is a few milliseconds long
/// and falls while the shell is still reading its profile, which is not where
/// anything that could run out of memory gets started. A layout's startup
/// command in particular is not typed until the first prompt has been drawn.
pub fn adopt(pid: std.c.pid_t) void {
const conn = bus orelse blk: {
var err: ?*glib.Error = null;
const found = gio.busGetSync(.session, null, &err) orelse {
defer if (err) |e| e.free();
warnOnce(if (err) |e| e.f_message else null);
return;
};
bus = found;
break :blk found;
};
// The random part keeps the name unique even if a scope outlives its shell,
// which it does whenever the shell left a daemon behind — Gradle's does this
// as a matter of course — and the pid is later reused.
var name_buf: [96]u8 = undefined;
const name = std.fmt.bufPrintZ(&name_buf, "app-playpen-shell-{d}-{x:0>8}.scope", .{
pid,
glib.randomInt(),
}) catch return;
// `CollectMode` because a scope in which something *was* OOM-killed ends up
// "failed" when it finally empties, and would otherwise sit in
// `systemctl --user --failed` long after the tab is gone.
const params = glib.Variant.newParsed(
"(%s, 'fail', [" ++
"('Description', <'Playpen shell'>), " ++
"('Slice', <'app.slice'>), " ++
"('PIDs', <[%u]>), " ++
"('OOMPolicy', <'continue'>), " ++
"('CollectMode', <'inactive-or-failed'>)" ++
"], @a(sa(sv)) [])",
name.ptr,
@as(u32, @intCast(pid)),
);
conn.call(
"org.freedesktop.systemd1",
"/org/freedesktop/systemd1",
"org.freedesktop.systemd1.Manager",
"StartTransientUnit",
params,
null,
.{},
-1,
null,
&onStarted,
null,
);
}
fn onStarted(source: ?*gobject.Object, res: *gio.AsyncResult, _: ?*anyopaque) callconv(.c) void {
const conn: *gio.DBusConnection = @ptrCast(source orelse return);
var err: ?*glib.Error = null;
const reply = conn.callFinish(res, &err) orelse {
defer if (err) |e| e.free();
warnOnce(if (err) |e| e.f_message else null);
return;
};
reply.unref();
}
fn warnOnce(message: ?[*:0]const u8) void {
if (warned) return;
warned = true;
std.log.warn(
"could not give shells their own systemd scope, so one running out of memory can take the app down with it: {s}",
.{message orelse "unknown error"},
);
}
+17 -1
View File
@@ -208,6 +208,10 @@ export default function App() {
// a repeat request for the same file a distinct one, so re-clicking a comment // a repeat request for the same file a distinct one, so re-clicking a comment
// after re-folding its file opens it again. // after re-folding its file opens it again.
const [reveal, setReveal] = useState<{ file: string; seq: number } | null>(null); const [reveal, setReveal] = useState<{ file: string; seq: number } | null>(null);
// The file the diff last asked the rail to show, sequenced like `reveal` so
// locating the same file twice still counts.
const [locate, setLocate] = useState<{ file: string; seq: number } | null>(null);
const locateSeq = useRef(0);
const revealSeq = useRef(0); const revealSeq = useRef(0);
const [theme, setTheme] = useState<Theme>(initialTheme); const [theme, setTheme] = useState<Theme>(initialTheme);
const [sidebarWidth, setSidebarWidth] = useState(() => const [sidebarWidth, setSidebarWidth] = useState(() =>
@@ -620,6 +624,11 @@ export default function App() {
waitForComment(c.id, () => scrollToFile(c.file)); waitForComment(c.id, () => scrollToFile(c.file));
}, []); }, []);
// Stable, since every file header holds it and FileView is memoized.
const locateFile = useCallback((file: string) => {
setLocate({ file, seq: ++locateSeq.current });
}, []);
const draftCount = useMemo( const draftCount = useMemo(
() => comments.filter((c) => c.status === 'draft').length, () => comments.filter((c) => c.status === 'draft').length,
[comments], [comments],
@@ -895,7 +904,13 @@ export default function App() {
/> />
)} )}
{payload && ( {payload && (
<FileList files={payload.files} comments={comments} onSelect={scrollToFile} /> <FileList
files={payload.files}
comments={comments}
viewed={viewed}
locate={locate}
onSelect={scrollToFile}
/>
)} )}
<div className="sidebar-foot"> <div className="sidebar-foot">
{openCount > 0 && ( {openCount > 0 && (
@@ -947,6 +962,7 @@ export default function App() {
snapshots={snapshots} snapshots={snapshots}
reveal={reveal} reveal={reveal}
onSetViewed={setFileViewed} onSetViewed={setFileViewed}
onLocate={locateFile}
onStartDraft={setDraft} onStartDraft={setDraft}
onCancelDraft={cancelDraft} onCancelDraft={cancelDraft}
onSubmitDraft={submitDraft} onSubmitDraft={submitDraft}
+14
View File
@@ -64,6 +64,8 @@ interface Props {
// `source` is the file's contents at the moment it was marked, kept so a later // `source` is the file's contents at the moment it was marked, kept so a later
// visit can be diffed against it. Absent when they can't be reconstructed. // visit can be diffed against it. Absent when they can't be reconstructed.
onSetViewed: (file: string, viewed: boolean, source?: string) => void; onSetViewed: (file: string, viewed: boolean, source?: string) => void;
// Reveals the file in the left rail's file list.
onLocate: (file: string) => void;
onStartDraft: (d: DraftTarget) => void; onStartDraft: (d: DraftTarget) => void;
onCancelDraft: () => void; onCancelDraft: () => void;
onSubmitDraft: (body: string) => Promise<void>; onSubmitDraft: (body: string) => Promise<void>;
@@ -432,6 +434,7 @@ export function DiffView({
snapshots, snapshots,
reveal, reveal,
onSetViewed, onSetViewed,
onLocate,
onStartDraft, onStartDraft,
onCancelDraft, onCancelDraft,
onSubmitDraft, onSubmitDraft,
@@ -480,6 +483,7 @@ export function DiffView({
cardRef={cardRef(path)} cardRef={cardRef(path)}
bodyRef={bodyRef(path)} bodyRef={bodyRef(path)}
onSetViewed={onSetViewed} onSetViewed={onSetViewed}
onLocate={onLocate}
onStartDraft={onStartDraft} onStartDraft={onStartDraft}
onCancelDraft={onCancelDraft} onCancelDraft={onCancelDraft}
onSubmitDraft={onSubmitDraft} onSubmitDraft={onSubmitDraft}
@@ -537,6 +541,7 @@ const FileView = memo(function FileView({
cardRef, cardRef,
bodyRef, bodyRef,
onSetViewed, onSetViewed,
onLocate,
onStartDraft, onStartDraft,
onCancelDraft, onCancelDraft,
onSubmitDraft, onSubmitDraft,
@@ -561,6 +566,7 @@ const FileView = memo(function FileView({
cardRef: (el: HTMLElement | null) => void; cardRef: (el: HTMLElement | null) => void;
bodyRef: (el: HTMLElement | null) => void; bodyRef: (el: HTMLElement | null) => void;
onSetViewed: Props['onSetViewed']; onSetViewed: Props['onSetViewed'];
onLocate: Props['onLocate'];
onStartDraft: (d: DraftTarget) => void; onStartDraft: (d: DraftTarget) => void;
onCancelDraft: () => void; onCancelDraft: () => void;
onSubmitDraft: Props['onSubmitDraft']; onSubmitDraft: Props['onSubmitDraft'];
@@ -749,6 +755,14 @@ const FileView = memo(function FileView({
> >
<Icon name="copy" /> <Icon name="copy" />
</button> </button>
<button
className="icon-btn"
onClick={() => onLocate(path)}
title="Show in file list"
aria-label="Show in file list"
>
<Icon name="focus-center" />
</button>
{file.type !== 'modify' && ( {file.type !== 'modify' && (
<span className={`file-status file-status-${file.type}`}> <span className={`file-status file-status-${file.type}`}>
{statusLabel(file.type)} {statusLabel(file.type)}
+73 -6
View File
@@ -1,4 +1,4 @@
import { useMemo, useState } from 'react'; import { useEffect, useMemo, useRef, useState } from 'react';
import type { Comment, DiffFile } from '../types'; import type { Comment, DiffFile } from '../types';
import { import {
@@ -8,12 +8,18 @@ import {
pathOf, pathOf,
type DirNode, type DirNode,
type FileNode, type FileNode,
type TreeNode,
} from '../lib/filetree'; } from '../lib/filetree';
import { Icon } from './Icon'; import { Icon } from './Icon';
interface Props { interface Props {
files: DiffFile[]; files: DiffFile[];
comments: Comment[]; comments: Comment[];
// Files marked viewed; their rows gray out, as do folders with nothing left.
viewed: ReadonlySet<string>;
// A file the diff asked to be shown here, sequenced so asking twice for the
// same file still counts.
locate: { file: string; seq: number } | null;
onSelect: (path: string) => void; onSelect: (path: string) => void;
} }
@@ -28,9 +34,12 @@ function initialMode(): Mode {
// FileList is the left rail: every changed file with its stats and open-comment // FileList is the left rail: every changed file with its stats and open-comment
// count, either as a GitHub-style collapsible folder tree or a flat list. // count, either as a GitHub-style collapsible folder tree or a flat list.
// Clicking a file scrolls to it. // Clicking a file scrolls to it.
export function FileList({ files, comments, onSelect }: Props) { export function FileList({ files, comments, viewed, locate, onSelect }: Props) {
const [mode, setMode] = useState<Mode>(initialMode); const [mode, setMode] = useState<Mode>(initialMode);
const [collapsed, setCollapsed] = useState<Set<string>>(new Set()); const [collapsed, setCollapsed] = useState<Set<string>>(new Set());
const navRef = useRef<HTMLElement>(null);
// The located file waiting for its row to be on screen to scroll to.
const [pending, setPending] = useState<string | null>(null);
const openByFile = useMemo(() => { const openByFile = useMemo(() => {
const m = new Map<string, number>(); const m = new Map<string, number>();
@@ -43,6 +52,7 @@ export function FileList({ files, comments, onSelect }: Props) {
const tree = useMemo(() => buildTree(files, openByFile), [files, openByFile]); const tree = useMemo(() => buildTree(files, openByFile), [files, openByFile]);
const rows = useMemo(() => flatten(tree, collapsed), [tree, collapsed]); const rows = useMemo(() => flatten(tree, collapsed), [tree, collapsed]);
const viewedDirs = useMemo(() => fullyViewed(tree, viewed), [tree, viewed]);
const allCollapsed = useMemo(() => { const allCollapsed = useMemo(() => {
const dirs = dirPaths(tree); const dirs = dirPaths(tree);
return dirs.length > 0 && dirs.every((p) => collapsed.has(p)); return dirs.length > 0 && dirs.every((p) => collapsed.has(p));
@@ -63,8 +73,40 @@ export function FileList({ files, comments, onSelect }: Props) {
const toggleAll = () => const toggleAll = () =>
setCollapsed(allCollapsed ? new Set() : new Set(dirPaths(tree))); setCollapsed(allCollapsed ? new Set() : new Set(dirPaths(tree)));
// Locating a file opens every folder above it. A folder's collapse key is its
// own path, so those are exactly the prefixes of the file's path — including
// the ones a chain-merged row swallowed, which were never keys to begin with.
useEffect(() => {
if (!locate) return;
const parts = locate.file.split('/');
const ancestors = parts.slice(0, -1).map((_, i) => parts.slice(0, i + 1).join('/'));
setCollapsed((prev) => {
if (!ancestors.some((a) => prev.has(a))) return prev;
const next = new Set(prev);
for (const a of ancestors) next.delete(a);
return next;
});
setPending(locate.file);
}, [locate]);
// Then, once the render that opened those folders has the row in the DOM,
// bring it into view and pulse it so the eye finds it.
useEffect(() => {
if (pending == null) return;
const el = navRef.current?.querySelector<HTMLElement>(
`[data-path="${CSS.escape(pending)}"]`,
);
if (!el) return;
setPending(null);
el.scrollIntoView({ behavior: 'smooth', block: 'center' });
el.classList.remove('is-flashed');
void el.offsetWidth; // restart the flash when the same row is located again
el.classList.add('is-flashed');
window.setTimeout(() => el.classList.remove('is-flashed'), 1800);
}, [pending, rows, mode]);
return ( return (
<nav className="filelist"> <nav className="filelist" ref={navRef}>
<div className="filelist-head"> <div className="filelist-head">
<span> <span>
{files.length} file{files.length === 1 ? '' : 's'} changed {files.length} file{files.length === 1 ? '' : 's'} changed
@@ -105,6 +147,7 @@ export function FileList({ files, comments, onSelect }: Props) {
}} }}
depth={0} depth={0}
showDir showDir
viewed={viewed.has(pathOf(f))}
onSelect={onSelect} onSelect={onSelect}
/> />
)) ))
@@ -115,6 +158,7 @@ export function FileList({ files, comments, onSelect }: Props) {
node={node} node={node}
depth={depth} depth={depth}
collapsed={collapsed.has(node.path)} collapsed={collapsed.has(node.path)}
viewed={viewedDirs.has(node.path)}
onToggle={() => toggleDir(node.path)} onToggle={() => toggleDir(node.path)}
/> />
) : ( ) : (
@@ -122,6 +166,7 @@ export function FileList({ files, comments, onSelect }: Props) {
key={node.path} key={node.path}
node={node} node={node}
depth={depth} depth={depth}
viewed={viewed.has(node.path)}
onSelect={onSelect} onSelect={onSelect}
/> />
), ),
@@ -131,6 +176,23 @@ export function FileList({ files, comments, onSelect }: Props) {
); );
} }
// fullyViewed collects the folders whose every file is marked viewed, so a
// folder you've finished steps back along with its files — and stays that way
// when it's folded away and its files can't say so themselves.
function fullyViewed(nodes: TreeNode[], viewed: ReadonlySet<string>): Set<string> {
const out = new Set<string>();
const walk = (n: TreeNode): boolean => {
if (n.kind === 'file') return viewed.has(n.path);
// Every child is walked, not just up to the first unviewed one: a folder
// deeper down can be done even when its parent isn't.
const done = n.children.map(walk).every(Boolean);
if (done) out.add(n.path);
return done;
};
nodes.forEach(walk);
return out;
}
// indent mirrors the tree depth; the chevron column keeps files aligned with // indent mirrors the tree depth; the chevron column keeps files aligned with
// the folder name above them. // the folder name above them.
function indent(depth: number) { function indent(depth: number) {
@@ -141,17 +203,19 @@ function DirRow({
node, node,
depth, depth,
collapsed, collapsed,
viewed,
onToggle, onToggle,
}: { }: {
node: DirNode; node: DirNode;
depth: number; depth: number;
collapsed: boolean; collapsed: boolean;
viewed: boolean;
onToggle: () => void; onToggle: () => void;
}) { }) {
return ( return (
<li> <li>
<button <button
className="filelist-dir-row" className={`filelist-dir-row${viewed ? ' is-viewed' : ''}`}
style={indent(depth)} style={indent(depth)}
onClick={onToggle} onClick={onToggle}
title={node.path} title={node.path}
@@ -189,11 +253,13 @@ function FileRow({
node, node,
depth, depth,
showDir = false, showDir = false,
viewed,
onSelect, onSelect,
}: { }: {
node: FileNode; node: FileNode;
depth: number; depth: number;
showDir?: boolean; showDir?: boolean;
viewed: boolean;
onSelect: (path: string) => void; onSelect: (path: string) => void;
}) { }) {
const name = showDir ? node.path.split('/').pop() : node.name; const name = showDir ? node.path.split('/').pop() : node.name;
@@ -202,10 +268,11 @@ function FileRow({
return ( return (
<li> <li>
<button <button
className="filelist-file-row" className={`filelist-file-row${viewed ? ' is-viewed' : ''}`}
style={indent(depth)} style={indent(depth)}
onClick={() => onSelect(node.path)} onClick={() => onSelect(node.path)}
title={node.path} title={viewed ? `${node.path} (viewed)` : node.path}
data-path={node.path}
> >
<span className="filelist-chevron" /> <span className="filelist-chevron" />
<span className={`filelist-icon is-${node.file.status}`}> <span className={`filelist-icon is-${node.file.status}`}>
+2
View File
@@ -72,6 +72,8 @@ const PATHS = {
'M3.75 2h3.5a.75.75 0 0 1 0 1.5h-3.5a.25.25 0 0 0-.25.25v8.5c0 .138.112.25.25.25h8.5a.25.25 0 0 0 .25-.25v-3.5a.75.75 0 0 1 1.5 0v3.5A1.75 1.75 0 0 1 12.25 14h-8.5A1.75 1.75 0 0 1 2 12.25v-8.5A1.75 1.75 0 0 1 3.75 2Zm6.854-1h3.396a.25.25 0 0 1 .25.25v3.396a.25.25 0 0 1-.427.177L12.5 3.561 8.53 7.53a.75.75 0 0 1-1.06-1.06l3.969-3.97-1.262-1.323a.25.25 0 0 1 .177-.427Z', 'M3.75 2h3.5a.75.75 0 0 1 0 1.5h-3.5a.25.25 0 0 0-.25.25v8.5c0 .138.112.25.25.25h8.5a.25.25 0 0 0 .25-.25v-3.5a.75.75 0 0 1 1.5 0v3.5A1.75 1.75 0 0 1 12.25 14h-8.5A1.75 1.75 0 0 1 2 12.25v-8.5A1.75 1.75 0 0 1 3.75 2Zm6.854-1h3.396a.25.25 0 0 1 .25.25v3.396a.25.25 0 0 1-.427.177L12.5 3.561 8.53 7.53a.75.75 0 0 1-1.06-1.06l3.969-3.97-1.262-1.323a.25.25 0 0 1 .177-.427Z',
pencil: pencil:
'M11.013 1.427a1.75 1.75 0 0 1 2.474 0l1.086 1.086a1.75 1.75 0 0 1 0 2.474l-8.61 8.61c-.21.21-.47.364-.756.445l-3.251.93a.75.75 0 0 1-.927-.928l.929-3.25c.081-.286.235-.547.445-.758l8.61-8.61Zm.176 4.823L9.75 4.81l-6.286 6.287a.253.253 0 0 0-.064.108l-.558 1.953 1.953-.558a.253.253 0 0 0 .108-.064Zm1.238-3.763a.25.25 0 0 0-.354 0L10.811 3.75l1.439 1.44 1.263-1.263a.25.25 0 0 0 0-.354Z', 'M11.013 1.427a1.75 1.75 0 0 1 2.474 0l1.086 1.086a1.75 1.75 0 0 1 0 2.474l-8.61 8.61c-.21.21-.47.364-.756.445l-3.251.93a.75.75 0 0 1-.927-.928l.929-3.25c.081-.286.235-.547.445-.758l8.61-8.61Zm.176 4.823L9.75 4.81l-6.286 6.287a.253.253 0 0 0-.064.108l-.558 1.953 1.953-.558a.253.253 0 0 0 .108-.064Zm1.238-3.763a.25.25 0 0 0-.354 0L10.811 3.75l1.439 1.44 1.263-1.263a.25.25 0 0 0 0-.354Z',
'focus-center':
'M2.75 2.5a.25.25 0 0 0-.25.25v2.5a.75.75 0 0 1-1.5 0v-2.5C1 1.784 1.784 1 2.75 1h2.5a.75.75 0 0 1 0 1.5h-2.5ZM10 1.75a.75.75 0 0 1 .75-.75h2.5c.966 0 1.75.784 1.75 1.75v2.5a.75.75 0 0 1-1.5 0v-2.5a.25.25 0 0 0-.25-.25h-2.5a.75.75 0 0 1-.75-.75ZM1.75 10a.75.75 0 0 1 .75.75v2.5c0 .138.112.25.25.25h2.5a.75.75 0 0 1 0 1.5h-2.5A1.75 1.75 0 0 1 1 13.25v-2.5a.75.75 0 0 1 .75-.75Zm12.5 0a.75.75 0 0 1 .75.75v2.5A1.75 1.75 0 0 1 13.25 15h-2.5a.75.75 0 0 1 0-1.5h2.5a.25.25 0 0 0 .25-.25v-2.5a.75.75 0 0 1 .75-.75ZM8 10a2 2 0 1 0 .001-3.999A2 2 0 0 0 8 10Z',
clock: clock:
'M8 0a8 8 0 1 1 0 16A8 8 0 0 1 8 0ZM1.5 8a6.5 6.5 0 1 0 13 0 6.5 6.5 0 0 0-13 0Zm7-3.25v2.992l2.028.812a.75.75 0 0 1-.557 1.392l-2.5-1A.751.751 0 0 1 7 8.25v-3.5a.75.75 0 0 1 1.5 0Z', 'M8 0a8 8 0 1 1 0 16A8 8 0 0 1 8 0ZM1.5 8a6.5 6.5 0 1 0 13 0 6.5 6.5 0 0 0-13 0Zm7-3.25v2.992l2.028.812a.75.75 0 0 1-.557 1.392l-2.5-1A.751.751 0 0 1 7 8.25v-3.5a.75.75 0 0 1 1.5 0Z',
} as const; } as const;
+29
View File
@@ -972,6 +972,35 @@ body.is-resizing {
.filelist-stats .del { .filelist-stats .del {
color: var(--del-fg); color: var(--del-fg);
} }
/* Viewed files step back here as they do in the diff, and so does a folder with
nothing left to read in it; hovering brings a row back to full strength. */
.filelist-dir-row.is-viewed,
.filelist-file-row.is-viewed {
opacity: 0.55;
}
.filelist-dir-row.is-viewed:hover,
.filelist-file-row.is-viewed:hover {
opacity: 1;
}
.filelist-file-row.is-viewed .filelist-name,
.filelist-dir-row.is-viewed .filelist-folder {
color: var(--muted);
}
/* Pulse when the diff asks the rail to show a file. */
.filelist-file-row.is-flashed {
animation: filelist-flash 1.8s ease-out;
}
@keyframes filelist-flash {
0%,
35% {
background: var(--accent-soft);
box-shadow: inset 0 0 0 1px var(--accent-emphasis);
opacity: 1;
}
100% {
box-shadow: inset 0 0 0 1px transparent;
}
}
/* GitHub marks a file's unresolved conversations with a comment glyph + count. */ /* GitHub marks a file's unresolved conversations with a comment glyph + count. */
.filelist-badge { .filelist-badge {
display: inline-flex; display: inline-flex;