Port ted settings UX from clet edit: immediate settings persistence via View/Options, no save-on-quit#123
Conversation
…tor settings Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/f94626f4-d942-433f-bce8-06c1b977241e Co-authored-by: tig <585482+tig@users.noreply.github.com>
…fig path Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/f94626f4-d942-433f-bce8-06c1b977241e Co-authored-by: tig <585482+tig@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/f94626f4-d942-433f-bce8-06c1b977241e Co-authored-by: tig <585482+tig@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18dd69adbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@copilot see cr comments and address |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/1857480b-56f0-4e86-9e77-c6285d88978f Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Addressed the CR feedback in b617477. Changes made:
Screenshot (updated ted View menu UI): |
|
@copilot ci/cd is failing. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/f5d77102-9405-46d4-bfc8-a7058a027789 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Fixed in 2912c52. CI was failing on ubuntu at No UI behavior changed in this fix; screenshot reference remains: |
|
@copilot bring the clet settings tests over. Right now, user testing shows settings are not being saved. E.g. I change tab indent and no ~/.tui/ted.config.json is created. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/fa3e24ac-04b4-4c7d-b1dd-37e170f564d8 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/fa3e24ac-04b4-4c7d-b1dd-37e170f564d8 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
@copilot ci cd is failing again. You must endure all tests pass before committing! |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/32fc12e0-fbae-4c4f-96fa-c989169fcfe1 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/32fc12e0-fbae-4c4f-96fa-c989169fcfe1 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/32fc12e0-fbae-4c4f-96fa-c989169fcfe1 Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Fixed in e450119. I investigated the failing CI jobs (macOS + Windows), reproduced the failure locally, and stabilized the flaky integration test that was failing ( I re-ran full local validation before committing:
Screenshot (no UI layout change): Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…gui-cs/Editor into copilot/port-settings-ui-to-ted
…gui-cs/Editor into copilot/port-settings-ui-to-ted
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f0726f6a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The wordWrapCheckBox.ValueChanged handler conflicted with the MenuItem's Action handler. When the Menu dispatches Activate to the MenuItem (Shortcut) with a binding context, the relay dispatch activates the CheckBox first (triggering ValueChanged → sets WordWrap=true + saves), then fires Action (which computes !WordWrap=false + saves). Net result: WordWrap reverts to false and the config file contains the wrong value. The fix removes the ValueChanged handler entirely. The Action handler already correctly toggles Editor.WordWrap, updates the CheckBox, and calls SaveViewSettings(). This matches the pattern used by the other View menu checkboxes (Line Numbers, Fold Indicators, ShowTabs, etc.). Added test ViewMenu_WordWrap_Toggle_Persists_True that: - Opens View menu via Alt+V (matching real user interaction) - Clicks Word Wrap menu item - Asserts Editor.WordWrap == true after toggle - Asserts config file is created with correct content The test fails without the fix (WordWrap reverts to false) and passes with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three bugs fixed: 1. Config path: GetConfigPath() used %APPDATA%\tui on Windows instead of ~/.tui (%USERPROFILE%\.tui). Now uses HOME env var with .tui subdirectory on all platforms, matching clet config/edit convention. 2. No explicit Load: settings were never read from ted.config.json on startup. TedApp constructor read EditorSettings statics which were at their C# defaults. Added EditorSettings.Load() that parses the JSON file and populates statics. Called from Program.cs before TedApp construction. 3. ConfigurationManager.Load+Apply after every save: Save() called ConfigurationManager.Load(All) + Apply() which could reset EditorSettings statics to defaults, causing cross-setting interference (e.g., ShowTabs and WordWrap undoing each other). Removed — the JSON file write is sufficient. Also: - Made GetConfigPath() internal for testability - Added InternalsVisibleTo for test project - Added Load_Reads_Settings_From_ConfigFile test - Added Load_TedApp_Applies_Persisted_WordWrap test - Simplified ConfigPathScope (no longer needs APPDATA redirect) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use NumericUpDown.ValueChanging to reject values below 1 instead of clamping after the fact in ApplyTo. The Math.Max(1, ...) guard in ApplyTo remains as a safety net. Updated test to verify the rejection behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89d2e27b6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Load_Reads_Settings_From_ConfigFile and Load_TedApp_Applies_Persisted_WordWrap modify EditorSettings static properties (e.g., WordWrap=true) but didn't restore defaults. On macOS/Linux where test execution order differs from Windows, this polluted subsequent tests (ViewMenu_WordWrap_Toggle_Persists_True expected WordWrap=false initially). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Merged Load_Reads_Settings_From_ConfigFile and Load_TedApp_Applies_Persisted_WordWrap into a single Load_TedApp_Applies_Persisted_Settings test that asserts via TedApp.Editor instance properties (not EditorSettings statics) - Moved EditorSettings statics reset into ConfigPathScope.Dispose so every test that uses a scope gets clean static state automatically - Removed standalone ResetEditorSettingsDefaults helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 504654a7a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ReadBool and ReadInt used unanchored regex that could match keys inside // comments, loading stale/wrong values from JSONC config files. Fixed by anchoring to start-of-line with a negative lookahead for leading //. Added Load_Skips_Commented_Out_Settings test. Addresses Codex review P2 findings on EditorSettings.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61bf634a52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- ReadBool/ReadInt: require key at JSON property position (only whitespace before quoted key), preventing false matches from keys embedded inside string values on the same line. - Save: use FindRootClosingBrace() that skips any '}' on lines starting with '//', preventing insertion into trailing JSONC comments. - Added regression tests for both scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f0493aeaf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.