feat: configurable autoclose when shell exits #1699
Conversation
WalkthroughThe pull request introduces new configuration options for terminal behavior across multiple files in the project. These changes enable more flexible terminal settings, specifically focusing on automatic closing mechanisms. The modifications span both frontend and backend components, adding new constants, type definitions, and configuration properties. The key additions include settings for automatic terminal closure, such as enabling auto-close, configuring closure on error, and setting a delay for automatic closure. These settings are implemented consistently across the configuration files, type definitions, and frontend store, providing a comprehensive approach to enhancing terminal interaction and management. The changes are implemented in a modular and extensible manner, allowing users to customize terminal behavior through global settings while maintaining the existing functionality of the application. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/app/store/keymodel.ts (1)
166-168: Consider using type-safe assertions and constant defaults.The type assertions and default values could be improved for better maintainability and type safety.
Consider this approach:
-const termAutoClose = globalStore.get(getSettingsKeyAtom("term:autoclose")) as boolean | undefined; -const termAutoCloseOnError = globalStore.get(getSettingsKeyAtom("term:autocloseonerror")) as boolean | undefined; -const termAutoCloseDelay = globalStore.get(getSettingsKeyAtom("term:autoclosedelay")) as number | undefined; +const DEFAULT_AUTO_CLOSE_DELAY = 2000; + +const termAutoClose = globalStore.get(getSettingsKeyAtom("term:autoclose")) as SettingsType["term:autoclose"]; +const termAutoCloseOnError = globalStore.get(getSettingsKeyAtom("term:autocloseonerror")) as SettingsType["term:autocloseonerror"]; +const termAutoCloseDelay = globalStore.get(getSettingsKeyAtom("term:autoclosedelay")) as SettingsType["term:autoclosedelay"];pkg/wconfig/defaultconfig/widgets.json (1)
10-12: Consider adding comments to explain the relationship between settings.The terminal widget configuration aligns with the new settings.json values, but the relationship between
settings.jsonand these widget properties could be clearer:
cmd:closeonexit: maps toterm:autoclosecmd:closeonexitforce: maps toterm:autocloseonerrorcmd:closeonexitdelay: maps toterm:autoclosedelayConsider adding JSON comments to document this mapping:
"meta": { "view": "term", "controller": "shell", + // Controlled by term:autoclose in settings.json "cmd:closeonexit": true, + // Controlled by term:autocloseonerror in settings.json "cmd:closeonexitforce": false, + // Controlled by term:autoclosedelay in settings.json "cmd:closeonexitdelay": 2000 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/app/store/keymodel.ts(2 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wconfig/defaultconfig/settings.json(1 hunks)pkg/wconfig/defaultconfig/widgets.json(1 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)
🔇 Additional comments (4)
pkg/wconfig/metaconsts.go (1)
38-40: LGTM! Well-organized constants.The new terminal auto-close configuration constants are properly named and grouped with other terminal-related settings.
pkg/wconfig/settingsconfig.go (1)
65-67: LGTM! Well-structured settings fields.The new settings fields are properly defined with:
- Pointer types for optional values
- Consistent naming
- Proper JSON tags with omitempty
frontend/types/gotypes.d.ts (1)
604-606: LGTM! Consistent type definitions.The TypeScript type definitions correctly mirror the Go struct fields and maintain type safety across the application.
pkg/wconfig/defaultconfig/widgets.json (1)
9-12: Verify the interaction between controller and cmd:close properties.*Let's ensure the "shell" controller properly handles these new close-on-exit properties.
✅ Verification successful
Properties are properly implemented and documented
The shell controller correctly handles all close-on-exit properties:
cmd:closeonexitcloses the block on successful exit (code 0)cmd:closeonexitforcecloses the block regardless of exit codecmd:closeonexitdelayproperly implements the configurable delayThe implementation in
pkg/blockcontroller/blockcontroller.gois complete and matches the documented behavior in the customwidgets documentation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for shell controller implementation and its handling of close-on-exit properties ast-grep --pattern 'class $_ implements Controller { $$$ shell($$$) { $$$ } $$$ }'Length of output: 96
Script:
#!/bin/bash # Find controller-related files fd -t f controller # Search for shell controller implementations or references rg -i "shell.*controller" -A 5 # Search for close-on-exit property handling rg -i "closeon(exit|Exit)" -A 3Length of output: 12015
| "cmd:closeonexitforce": termAutoCloseOnError ?? false, | ||
| "cmd:closeonexitdelay": termAutoCloseDelay ?? 2000, |
There was a problem hiding this comment.
💡 Codebase verification
Add the missing terminal auto-close setting
The cmd:closeonexit setting should be added as suggested. This is consistent with:
- The default configuration in
pkg/wconfig/defaultconfig/widgets.json - The documented behavior where this setting specifically handles auto-closing on successful exit
- The pattern of using all three close-related settings together
🔗 Analysis chain
Use the extracted settings consistently.
The termAutoClose setting is retrieved but not used in the block definition.
Consider if this setting should affect the block definition:
meta: {
view: "term",
controller: "shell",
"cmd:closeonexitforce": termAutoCloseOnError ?? false,
"cmd:closeonexitdelay": termAutoCloseDelay ?? 2000,
+ "cmd:closeonexit": termAutoClose ?? true,
},🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find termAutoClose definition and usage
rg "termAutoClose" -A 3 -B 3
# Search for terminal close-related settings
rg "cmd:closeonexit" -A 2 -B 2
# Search for similar block definitions
ast-grep --pattern 'meta: {
view: "term",
$$$
}'
Length of output: 6226
| "term:copyonselect": true, | ||
| "term:autoclose": true, | ||
| "term:autocloseonerror": false, | ||
| "term:autoclosedelay": 2000 |
There was a problem hiding this comment.
💡 Codebase verification
Terminal auto-close settings need documentation
While the auto-close feature is mentioned in the README, the specific configuration options (term:autoclose, term:autocloseonerror, term:autoclosedelay) are not documented. Consider:
- Adding these settings to the user documentation
- Including descriptions of their purpose and effects
- Documenting the default values and their rationale
🔗 Analysis chain
LGTM! Terminal autoclose settings are well-structured.
The new terminal settings follow consistent naming conventions and provide sensible defaults:
term:autoclose: true (enables the feature by default)term:autocloseonerror: false (prevents unexpected closures on errors)term:autoclosedelay: 2000 (provides reasonable 2-second delay)
Let's verify these settings are properly documented:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these settings are documented in README or docs
rg -i "term:autoclose|term:autocloseonerror|term:autoclosedelay" docs/ README.md || echo "Settings documentation not found"
Length of output: 157
Script:
#!/bin/bash
# Search for any markdown files containing terminal-related documentation
fd -e md | xargs rg -i "terminal|term" -A 2 -B 2
# Look for schema files and check terminal settings
fd "schema|config" -e json | xargs rg -i "term:autoclose|term:autocloseonerror|term:autoclosedelay" -A 2 -B 2
# Check for comments near the settings definition
rg -C 5 "term:autoclose" pkg/wconfig/defaultconfig/settings.json
Length of output: 13695
#1530
Configs in
settings.jsonandwidgets.jsonfor terminals created byCMD + Nand by clicking the "Terminal" button.autoclose.mov