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
pull from: fix/editor-terminal-size
merge into: devs:master
devs:master
devs:feat/ai-api-type
devs:ci/docker-hub-publishing
devs:feat/disco-ac
devs:feat/privacy-enhancements
devs:fix/clientid-regression
devs:fix/ai-chat-completions-followup
devs:feat/ai-custom
devs:fix/unencrypted-send
devs:rollback/pre-upstream-merge
devs:fix/autoping-warning-null-domain
devs:fix/pad-dead-space-reclaim
devs:feat/autoping-warning
devs:chore/untrack-gitversion
devs:fix/multiline-pad-clip
devs:fix/issue-112-followups
devs:fix/issue-128-migrate-v3-dedup
devs:fix/delay-timestamp-validation
devs:ref/light-cleanup
devs:fix/history-scroll-pad-redraw-storm
devs:fix/plugin-post-display-incoming-only
devs:merge/upstream-full
devs:merge-improve
devs:chore/remove-chatlog-stage-1
devs:fix/ai-json-encoding
devs:feat/no-db-backlog-114
devs:fix/scroll-non-chat-windows
devs:fix/paged-non-chat-windows
devs:fix/ai-leaks
devs:feat/ai
devs:fix/ai-followups
devs:test/ai-coverage-unit-only
devs:test/ai-coverage
devs:refactor/scroll-mechanism
devs:fix/verify-per-contact-context
devs:feat/no-db-mode
devs:feat/ai-json
devs:feat/pikaur-parity-arch
devs:fix-pikaur-build
devs:feat/upstream-sync
devs:feat/functest-speedup
devs:fix/cwe-134-format-string-audit
devs:ci/separate-build-step
devs:test/autoping-functional-tests
devs:test/db-functional-tests
devs:fix/arch-build
devs:fix/xep-0030-disco-items-error-handling
devs:fix/xep-0030-empty-disco-items
devs:playground/fix/src_refactoring
devs:tests/disco
devs:fix/test-CI-stability
devs:feat/parallel-tests-clean
devs:feat/parallel-functional-tests
devs:fix/functional_tests_v2
devs:fix/functional_tests
devs:fix/connect_max_args
devs:feat/extended_debug_info
devs:playground/fix/scroll-stuck
devs:build/multicore
devs:build/reenable-fedora
devs:build/autoupdate
No Reviewers
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No project
Plans (2025-2026)
No Assignees
jabber.developer2
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: devs/cproof#153
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
No description provided.
Delete Branch "fix/editor-terminal-size"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.
410d1d803dto24dbd3a2e2Generally, 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* datui_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 environCould 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.
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
24dbd3a2e2to1ac58222b8View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.