Repository navigation
feat(command-palette): add a toggle editor word wrap command (closes #1036) - #1076
Conversation
The command palette gains an `editor.toggle-word-wrap` command that flips
the existing `editorWordWrap` preference, so wrapping applies to every SQL
editor immediately and persists like the Settings -> Appearance toggle.
- `src/utils/builtInCommands.ts`: new command item (label reflects the
current state: "Enable word wrap" / "Disable word wrap", keywords
`wrap`, `line`, `editor`). The built-in factory takes an optional
`editorActions` argument (`wordWrapEnabled` + `toggleWordWrap`) that the
hook supplies from live settings.
- `src/hooks/useCommandPaletteActionItems.ts`: reads the settings context
(directly via `useContext`, so the palette still renders outside a
provider) and wires the toggle through `updateSetting("editorWordWrap", ...)`.
- i18n: `commandPalette.commands.toggleWordWrapEnable/toggleWordWrapDisable`
added to all 11 locales.
- Tests: 4 new cases in `tests/utils/builtInCommands.test.ts` (presence,
state-dependent label, action execution, absent without editor actions);
the built-in command list expectation in
`tests/contexts/CommandPaletteProvider.test.tsx` includes the new command.
Closes #1036
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const wordWrapEnabled = settingsContext?.settings.editorWordWrap ?? true; | ||
| const toggleWordWrap = useCallback(() => { | ||
| if (!settingsContext) return; | ||
| settingsContext.updateSetting("editorWordWrap", !wordWrapEnabled); |
There was a problem hiding this comment.
SUGGESTION: Toggle writes a captured snapshot instead of the functional updater
updateSetting explicitly supports an updater form for read-then-write correctness (see the overload docs in src/contexts/SettingsContext.ts), and a toggle is a read-then-write: toggleWordWrap captures wordWrapEnabled in its closure, so two executions before a re-render (or any stale closure) write back the stale value. The updater form is race-safe and drops the captured-snapshot dependency:
| settingsContext.updateSetting("editorWordWrap", !wordWrapEnabled); | |
| settingsContext.updateSetting("editorWordWrap", (prev) => !(prev ?? true)); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "openConnections": "Ouvrir le gestionnaire de connexions", | ||
| "openTableInConsole": "Ouvrir la table actuelle dans la console SQL" | ||
| "openTableInConsole": "Ouvrir la table actuelle dans la console SQL", | ||
| "toggleWordWrapEnable": "Activer le retour a la ligne", |
There was a problem hiding this comment.
SUGGESTION: Missing accent — "retour a la ligne" should be "retour à la ligne"
The same file already uses the accented form for this feature ("Retour à la ligne automatique", fr.json:601); correct French requires "à".
| "toggleWordWrapEnable": "Activer le retour a la ligne", | |
| "toggleWordWrapEnable": "Activer le retour à la ligne", |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "openTableInConsole": "Ouvrir la table actuelle dans la console SQL" | ||
| "openTableInConsole": "Ouvrir la table actuelle dans la console SQL", | ||
| "toggleWordWrapEnable": "Activer le retour a la ligne", | ||
| "toggleWordWrapDisable": "Desactiver le retour a la ligne" |
There was a problem hiding this comment.
SUGGESTION: Missing accents — "Desactiver le retour a la ligne" should be "Désactiver le retour à la ligne"
Matches the established term "Retour à la ligne automatique" (fr.json:601).
| "toggleWordWrapDisable": "Desactiver le retour a la ligne" | |
| "toggleWordWrapDisable": "Désactiver le retour à la ligne" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "openConnections": "Abrir gestor de conexiones", | ||
| "openTableInConsole": "Abrir la tabla actual en la consola SQL" | ||
| "openTableInConsole": "Abrir la tabla actual en la consola SQL", | ||
| "toggleWordWrapEnable": "Activar ajuste de linea", |
There was a problem hiding this comment.
SUGGESTION: Missing accent — "linea" should be "línea"
The same file's word-wrap setting is "Ajuste de Línea" (es.json:596); Spanish requires the accent on "línea".
| "toggleWordWrapEnable": "Activar ajuste de linea", | |
| "toggleWordWrapEnable": "Activar ajuste de línea", |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "openTableInConsole": "Abrir la tabla actual en la consola SQL" | ||
| "openTableInConsole": "Abrir la tabla actual en la consola SQL", | ||
| "toggleWordWrapEnable": "Activar ajuste de linea", | ||
| "toggleWordWrapDisable": "Desactivar ajuste de linea" |
There was a problem hiding this comment.
SUGGESTION: Missing accent — "linea" should be "línea"
Matches the established term "Ajuste de Línea" (es.json:596).
| "toggleWordWrapDisable": "Desactivar ajuste de linea" | |
| "toggleWordWrapDisable": "Desactivar ajuste de línea" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "openTableInConsole": "현재 테이블을 SQL 콘솔에서 열기" | ||
| "openTableInConsole": "현재 테이블을 SQL 콘솔에서 열기", | ||
| "toggleWordWrapEnable": "줄 바꿈 활성화", | ||
| "toggleWordWrapDisable": "줄 비활성화" |
There was a problem hiding this comment.
SUGGESTION: Disable label drops "바꿈" (wrap) — inconsistent with the enable label
"줄 비활성화" reads as "disable line"; the enable label is "줄 바꿈 활성화" and the established term is "자동 줄바꿈" (ko.json:546). Use the parallel form:
| "toggleWordWrapDisable": "줄 비활성화" | |
| "toggleWordWrapDisable": "줄 바꿈 비활성화" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous Review Summary (commit 8fa1474)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8fa1474)Status: 6 Issues Found | Recommendation: Address suggestions before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (15 files)
Reviewed by free · Input: 91.8K · Output: 1.7K · Cached: 99.4K |
…p toggle - Hook: use the functional `updateSetting` updater instead of a captured snapshot, so a toggle fired while another write is in flight cannot overwrite it with a stale value. - fr: restore accents in "retour à la ligne" (both labels). - es: restore the accent in "ajuste de línea" (both labels). - ko: "줄 바꿈 비활성화" to match the enable label's "줄 바꿈 활성화".
debba
left a comment
debba
left a comment
There was a problem hiding this comment.
Hi @Furox-Art, thanks a lot for picking up #1036! Using the functional updater in updateSetting("editorWordWrap", (prev) => !(prev ?? true)) is the right call to avoid a stale value, and the comment explaining why will save the next person some head scratching. Nice job keeping createBuiltInCommandItems testable by injecting editorActions, and on writing real translations for all 11 locales.
Since this is your first PR here: welcome! If you feel like hanging out with us, you're very welcome on our Discord: https://discord.com/invite/K2hmhfHRSt
I'm approving this. A few small things you can pick up as a follow-up, none of them blocking:
- In
useCommandPaletteActionItems.tstheeditorActionsobject is always passed, even when there's noSettingsProvider. In that case the command still shows up buttoggleWordWrapdoes nothing. PassingeditorActionsonly whensettingsContextis available would be a bit cleaner (in the app the provider is always there, so this only shows up in isolated tests). SettingsProviderbuilds a newvalueobject on every render, so with[settingsContext]as dependency the itemsuseMemorecomputes on any settings change. The cost is negligible, just worth knowing.- The command is listed even when no editor is open. That's fine for a global preference, just mentioning it in case you'd rather scope it.
I'll still do a manual check on my side before merging.
Closes #1036
What
A Toggle word wrap command in the command palette that flips the existing
editorWordWrappreference — the change applies to every SQL editor immediately and persists like the Settings → Appearance toggle. The label reflects the current state ("Disable word wrap" when on, "Enable word wrap" when off), withwrap,line,editorkeywords.Where
src/utils/builtInCommands.ts— the command item lives next toconnection.new-consolein the editor group. The factory takes an optionaleditorActionsargument ({ wordWrapEnabled, toggleWordWrap }) so the pure command builder stays testable.src/hooks/useCommandPaletteActionItems.ts— reads the settings context and wires the toggle throughupdateSetting("editorWordWrap", …), same asAppearanceTab.tsx. The context is read viauseContextso the palette still renders outside aSettingsProvider(the app always provides one).commandPalette.commands.toggleWordWrapEnable/toggleWordWrapDisableadded to all 11 locales (en, de, es, fr, it, ja, ko, pt-BR, ru, tl, zh).Acceptance criteria
updateSettingpath as the Appearance toggle, which open editors read via context)tests/utils/builtInCommands.test.ts— 4 new cases: command presence + keywords, state-dependent label in both directions, action execution, and absence when editor actions are not provided. The palette provider test's command-list expectation includes the new command.Checks
pnpm typecheck— cleanpnpm lint— eslint + theme token check cleanpnpm test— 5859 passed; the single failure (tests/themes/compatibilityBaseline.test.ts) is pre-existing onmainand unrelated (verified by re-running it on a clean checkout)Disclosure: authored by @Furox-Art (AI-assisted). Happy to adjust anything.