-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add $EDITOR functionality #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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") |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(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") |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(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) |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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