Feat 441 writing assistant checkbox for feeding current editor contents to llm #474
Conversation
- Add checkbox to automatically include entire file content when no text is selected - Store auto-context preference in electron-store - Fix undefined message error in currentConversation display - Add proper type safety and null checks - Update UI with clear labeling for auto-context feature
- Add checkbox to automatically include entire file content when no text is selected - Store auto-context preference in electron-store - Fix undefined message error in currentConversation display - Add proper type safety and null checks - Update UI with clear labeling for auto-context feature
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Implements a new auto-context feature for the writing assistant that automatically includes full file content when no text is selected, with persistent user preferences stored in electron-store.
- Added
autoContextboolean field toStoreSchemain/electron/main/electron-store/storeConfig.tsfor persistent storage - Implemented IPC handlers
set-auto-contextandget-auto-contextin/electron/main/electron-store/ipcHandlers.ts - Added checkbox UI with proper accessibility in
/src/components/WritingAssistant/ConversationHistory.tsx - Inconsistent IPC handler pattern in
/electron/preload/index.tsdeviates fromcreateIPCHandlerusage, potentially affecting type safety - Modified text replacement logic in
/src/components/WritingAssistant/WritingAssistant.tsxto handle both selected and full-file contexts with proper null checks
5 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
| store.set(StoreKeys.AutoContext, autoContext) | ||
| event.sender.send('auto-context-changed', autoContext) |
There was a problem hiding this comment.
style: Consider adding error handling in case the store.set() operation fails
| ipcMain.handle('set-auto-context', (event, autoContext: boolean) => { | ||
| store.set(StoreKeys.AutoContext, autoContext) | ||
| event.sender.send('auto-context-changed', autoContext) | ||
| }) |
There was a problem hiding this comment.
logic: event.sender.send() is called after store.set() - if store.set() fails, the UI could get out of sync with the actual stored value
| ipcMain.handle('get-auto-context', () => { | ||
| return store.get(StoreKeys.AutoContext, true) // Default to true | ||
| }) |
There was a problem hiding this comment.
style: Missing type annotation for the return value which should be boolean
| @@ -82,4 +83,5 @@ export enum StoreKeys { | |||
| SpellCheck = 'spellCheck', | |||
| EditorFlexCenter = 'editorFlexCenter', | |||
| showDocumentStats = 'showDocumentStats', | |||
| AutoContext = 'autoContext', | |||
There was a problem hiding this comment.
logic: Key name 'autoContext' in enum differs from schema property 'AutoContext' - should match case
| const savedAutoContext = await window.electronStore.getAutoContext() | ||
| setAutoContext(savedAutoContext) | ||
| } |
There was a problem hiding this comment.
logic: savedAutoContext could be undefined on first load - should provide a default value of true to match initial state
| if (autoContext && !selectedText && editor) { | ||
| selectedText = editor.state.doc.textBetween(0, editor.state.doc.content.size) | ||
| } |
There was a problem hiding this comment.
style: large files could cause performance issues when getting entire document content - consider adding a size limit check
| const $pos = editor.state.doc.resolve(from) | ||
| if (!$pos?.parent?.textContent?.startsWith(' ')) { | ||
| resetSpaceTrigger() |
There was a problem hiding this comment.
style: optional chaining on textContent could mask underlying issues - consider more explicit null checks
- Add checkbox to automatically include entire file content when no text is selected - Store auto-context preference in electron-store - Fix undefined message error in currentConversation display - Add proper type safety and null checks - Update UI with clear labeling for auto-context feature
|
This looks awesome! But would it be better to place the toggler in this window instead of where the response is already being generated? |
…ngAssistant - Remove autoContext checkbox and related props from ConversationHistory component - Add autoContext checkbox to WritingAssistant's options container - Update component interfaces and prop passing - Maintain existing autoContext functionality and styling
Yes. Fixed it.
|
fixes #441
Evidence:
Updated:
https://github.com/user-attachments/assets/28ff5089-72e7-4d69-ae4a-392c5784cb33
Old:
https://github.com/user-attachments/assets/c8848edf-d89f-40bc-b0c3-bcf50c07ecbe
/claim #441