feat: add $EDITOR functionality#76
Conversation
- Implement external editor integration for card creation/editing - Add editor configuration support via $EDITOR env variable - Create temporary markdown files to prevent invalid cards being added - Update README with example usage
There was a problem hiding this comment.
Pull Request Overview
This PR adds editor integration functionality to enable users to create and edit flashcards using their preferred external editor via the $EDITOR environment variable.
- Adds keyboard shortcuts 'a' and 'e' for adding and editing cards during study sessions
- Implements editor process launching and response handling with temporary file management
- Extends the data store with functionality to add new cards to decks
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/data/editor.go | New module implementing editor process launching, temporary file creation, and card template generation |
| internal/data/editor_test.go | Comprehensive test suite for editor functionality including shell editor detection and temporary file operations |
| internal/data/store.go | Adds AddCardToDeck method to support adding new cards to existing decks |
| internal/ui/study_screen.go | Integrates editor functionality with new keybindings and response handling during study sessions |
| internal/ui/study_screen_test.go | Test cases for editor response handling in study screen context |
| README.md | Updates keyboard shortcuts documentation to include new 'a' and 'e' commands |
| t.Fatal("Couldn't create temp file") | ||
| } | ||
| defer os.Remove(tempFile.Name()) | ||
| if _, err := tempFile.WriteString(mockValidCardContent); err != nil { | ||
| t.Fatalf("Couldnt write to temp file") |
There was a problem hiding this comment.
The error message contains a spelling error: "Couldn't" should be "Couldn't" or "Could not".
| t.Fatal("Couldn't create temp file") | |
| } | |
| defer os.Remove(tempFile.Name()) | |
| if _, err := tempFile.WriteString(mockValidCardContent); err != nil { | |
| t.Fatalf("Couldnt write to temp file") | |
| t.Fatal("Could not create temp file") | |
| } | |
| defer os.Remove(tempFile.Name()) | |
| if _, err := tempFile.WriteString(mockValidCardContent); err != nil { | |
| t.Fatalf("Could not write to temp file") |
There was a problem hiding this comment.
(Optional)
You can setup pre-commits that will catch spelling errors before you commit: make pre-commit-setup.
Except from Makefile:
.PHONY: pre-commit-setup
pre-commit-setup:
@echo "Setting up pre-commit hooks..."
@echo "consider running <pre-commit autoupdate> to get the latest versions"
pre-commit install
pre-commit install --install-hooks
pre-commit run --all-files| } | ||
| defer os.Remove(tempFile.Name()) | ||
| if _, err := tempFile.WriteString(mockValidCardContent); err != nil { | ||
| t.Fatalf("Couldnt write to temp file") |
There was a problem hiding this comment.
The error message contains a spelling error: "Couldnt" should be "Couldn't" or "Could not".
| t.Fatalf("Couldnt write to temp file") | |
| t.Fatalf("Couldn't write to temp file") |
There was a problem hiding this comment.
(Optional)
You can setup pre-commits that will catch spelling errors before you commit: make pre-commit-setup.
Except from Makefile:
.PHONY: pre-commit-setup
pre-commit-setup:
@echo "Setting up pre-commit hooks..."
@echo "consider running <pre-commit autoupdate> to get the latest versions"
pre-commit install
pre-commit install --install-hooks
pre-commit run --all-files| // Create Fake File | ||
| tempDir, err := os.MkdirTemp("", "card-test") | ||
| if err != nil { | ||
| t.Fatalf("Cant cretae temp dir: %v", err) |
There was a problem hiding this comment.
The error message contains spelling errors: "Cant" should be "Can't" or "Cannot", and "cretae" should be "create".
| case data.EditorResponse: | ||
| err := s.handleEditorResponse(msg) | ||
| if err != nil { | ||
| fmt.Printf("editor error: %v", err) |
There was a problem hiding this comment.
Error handling uses fmt.Printf which outputs to stdout and doesn't provide feedback to the user in the TUI. Consider using a proper error display mechanism within the UI framework.
| } | ||
|
|
||
| func (s *StudyScreen) handleEditorResponse(msg data.EditorResponse) error { | ||
| defer os.Remove(msg.FileName) // All cases should cleanup temp file |
There was a problem hiding this comment.
The defer statement will always execute, even when there are early returns due to errors. This could lead to attempting to remove a file that may not exist or may have already been removed, potentially causing silent failures.
| defer os.Remove(msg.FileName) // All cases should cleanup temp file | |
| defer func() { | |
| if err := os.Remove(msg.FileName); err != nil { | |
| fmt.Printf("failed to remove temp file %s: %v\n", msg.FileName, err) | |
| } | |
| }() // All cases should cleanup temp file |
| editor, err := getShellEditor() | ||
|
|
||
| if err != nil { | ||
| return nil // can't launch |
There was a problem hiding this comment.
When getShellEditor fails, LaunchEditor returns nil without any error indication to the caller, making it difficult to distinguish between successful completion and failure.
| return nil // can't launch | |
| return func() tea.Msg { | |
| return EditorResponse{ | |
| FileName: file, | |
| ExitCode: err, | |
| IsEdit: isEdit, | |
| CardID: cardID, | |
| } | |
| } |
|
Is is possible to get a demo via AsciiNema? |
Adds integration with external editor via the $EDITOR environment variable to create and edit flashcards