fix(editor): follow live terminal size in external editor #153

Open
jabber.developer2 wants to merge 1 commits from fix/editor-terminal-size into master
Collaborator

The external compose editor (/editor) opened at the terminal size profanity had at startup, not the current one: after resizing the terminal, a curses editor (nano, vim, …) drew in the old dimensions and left the rest of the window black.

Cause. The editor is spawned via fork+execvp and inherits profanity's LINES/COLUMNS, set once at init and never refreshed. A curses editor honors those over the real terminal. profanity's own UI is fine — ui_resize() reads the live size via ioctl(TIOCGWINSZ) and ignores the stale env.

Fix. Assemble a copy of the environment without LINES/COLUMNS in the parent, then swap the child's environ to it before execvp, so the editor's curses falls back to ioctl(TIOCGWINSZ). Building the env pre-fork keeps the child's post-fork path to a single pointer assignment — async-signal-safe, unlike unsetenv() — which matters since profanity is multithreaded. The parent's environment is untouched.

Testing. Clean -Werror build; editor.o references environ, no unsetenv. Manual: launch in a small terminal, maximize, /editor → nano now fills the whole window.

The external compose editor (/editor) opened at the terminal size profanity had at startup, not the current one: after resizing the terminal, a curses editor (nano, vim, …) drew in the old dimensions and left the rest of the window black. Cause. The editor is spawned via fork+execvp and inherits profanity's LINES/COLUMNS, set once at init and never refreshed. A curses editor honors those over the real terminal. profanity's own UI is fine — ui_resize() reads the live size via ioctl(TIOCGWINSZ) and ignores the stale env. Fix. Assemble a copy of the environment without LINES/COLUMNS in the parent, then swap the child's environ to it before execvp, so the editor's curses falls back to ioctl(TIOCGWINSZ). Building the env pre-fork keeps the child's post-fork path to a single pointer assignment — async-signal-safe, unlike unsetenv() — which matters since profanity is multithreaded. The parent's environment is untouched. Testing. Clean -Werror build; editor.o references environ, no unsetenv. Manual: launch in a small terminal, maximize, /editor → nano now fills the whole window.
jabber.developer2 added 1 commit 2026-07-04 11:36:51 +00:00
fix(editor): follow live terminal size in external editor
All checks were successful
CI Code / Check coding style (pull_request) Successful in 31s
CI Code / Check spelling (pull_request) Successful in 1m6s
CI Code / Linux (ubuntu) (pull_request) Successful in 9m59s
CI Code / Linux (debian) (pull_request) Successful in 10m16s
CI Code / Code Coverage (pull_request) Successful in 10m30s
CI Code / Linux (arch) (pull_request) Successful in 14m9s
410d1d803d
The compose editor is spawned via fork+execvp and inherits profanity's
environment, including LINES/COLUMNS. Those hold the terminal size
captured when profanity started (set by readline/ncurses at init) and
are never refreshed on resize. A curses-based editor (nano, vim, ...)
honors LINES/COLUMNS over the real window, so it renders at the
launch-time dimensions and leaves the rest of the window unused after
the terminal has been resized.

Build a copy of the environment without LINES/COLUMNS before forking and
point the child's environ at it, so the editor's curses falls back to
ioctl(TIOCGWINSZ) and tracks the current window. The env is assembled in
the parent so the child only swaps the environ pointer, which is
async-signal-safe -- unlike unsetenv() -- in the multithreaded
fork->exec window. profanity itself is unaffected: it already reads the
live size via ioctl in ui_resize(), and its own environment is intact.
jabber.developer2 force-pushed fix/editor-terminal-size from 410d1d803d to 24dbd3a2e2 2026-07-04 11:37:02 +00:00 Compare
jabber.developer requested review from jabber.developer 2026-07-06 09:45:47 +00:00
jabber.developer added the
Reviewed
Confirmed
1
label 2026-07-06 09:45:54 +00:00
jabber.developer added this to the Plans (2025-2026) project 2026-07-06 09:46:00 +00:00
jabber.developer2 was assigned by jabber.developer 2026-07-06 09:46:06 +00:00
jabber.developer reviewed 2026-07-06 09:53:46 +00:00
jabber.developer left a comment
Owner

Generally, LGTM, nice catch and smart implementation. I have only a single concern regarding potential bug. Also, "async-signal-safe" maybe slightly misleading, as we don't have atomicity here. While it's safe, this explicit statement in commit message might cause wrong assumptions. Thanks for the PR.

Generally, LGTM, nice catch and smart implementation. I have only a single concern regarding potential bug. Also, "async-signal-safe" maybe slightly misleading, as we don't have atomicity here. While it's safe, this explicit statement in commit message might cause wrong assumptions. Thanks for the PR.
@@ -148,12 +166,15 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
ui_resize();
cons_show_error("Failed to start editor: %s", strerror(errno));
g_strfreev(editor_argv);
g_free(editor_env); // array only; strings are borrowed from environ

Could it get freed prior to COW completion on fork?

Could it get freed prior to COW completion on fork?
Author
Collaborator

Could it get freed prior to COW completion on fork?

Should be safe. After fork() the child has its own address space, so the
array (and the borrowed string pointers in it) is the child's private
copy-on-write copy. g_free() runs only in the parent — that write faults
in a private page for the parent and leaves the child's copy intact. The
child reads environ only up to execvp(), which then replaces the whole
address space (the kernel snapshots argv/envp first), so it never touches
anything the parent freed, regardless of scheduling order. Only the array
is freed, never the strings — they stay owned by the original environ.

"async-signal-safe" might cause wrong assumptions

Agreed, reworded. The intent wasn't to claim the pointer store itself
buys any guarantee, only that the child avoids unsetenv(), whose
allocator/locking work is the part that's unsafe to run after fork in a
multithreaded process. U

> Could it get freed prior to COW completion on fork? Should be safe. After fork() the child has its own address space, so the array (and the borrowed string pointers in it) is the child's private copy-on-write copy. g_free() runs only in the parent — that write faults in a private page for the parent and leaves the child's copy intact. The child reads environ only up to execvp(), which then replaces the whole address space (the kernel snapshots argv/envp first), so it never touches anything the parent freed, regardless of scheduling order. Only the array is freed, never the strings — they stay owned by the original environ. > "async-signal-safe" might cause wrong assumptions Agreed, reworded. The intent wasn't to claim the pointer store itself buys any guarantee, only that the child avoids unsetenv(), whose allocator/locking work is the part that's unsafe to run after fork in a multithreaded process. U
jabber.developer moved this to Verification in Plans (2025-2026) on 2026-07-11 21:39:46 +00:00
jabber.developer2 force-pushed fix/editor-terminal-size from 24dbd3a2e2 to 1ac58222b8 2026-07-13 12:34:38 +00:00 Compare
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
Required
Details
CI Code / Check coding style (pull_request) Successful in 23s
Required
Details
CI Code / Code Coverage (pull_request) Successful in 3m26s
Required
Details
CI Code / Linux (debian) (pull_request) Successful in 5m19s
Required
Details
CI Code / Linux (ubuntu) (pull_request) Successful in 5m23s
Required
Details
CI Code / Linux (arch) (pull_request) Successful in 7m17s
Required
Details
This pull request is blocked because it's outdated.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/editor-terminal-size:fix/editor-terminal-size
git checkout fix/editor-terminal-size
Sign in to join this conversation.
No description provided.