Dev/andreyt/better console#24
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a comprehensive console UI to the AudiobookGenerator application using the Spectre.Console library. The changes introduce an interactive command-line interface with menus for editing book metadata, chapters, images, voice selection, and audiobook generation. The PR also refactors the BookConverter class to expose its Parser and Synthesizer dependencies publicly, updates the WPF application to use dependency injection more cleanly, and upgrades the target framework from .NET 9 to .NET 10.
Changes:
- Added Spectre.Console-based interactive UI for the console application with commands: open, convert, voices, info, and help
- Refactored BookConverter to expose Parser and Synthesizer properties and made it sealed
- Updated WPF ViewModel to accept BookConverter via DI instead of creating it directly
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/AudiobookGenerator.Core/BookConverter.cs | Made class sealed and exposed Parser/Synthesizer properties publicly |
| src/AudiobookGenerator.Wpf/ViewModels/AudiobookGeneratorViewModel.cs | Removed manual BookConverter instantiation, uses DI, removed unused Logs collection |
| src/AudiobookGenerator.Console/Program.cs | Complete rewrite with command-line argument parsing and interactive/non-interactive modes |
| src/AudiobookGenerator.Console/Models/BookEditSession.cs | New mutable wrapper for Book editing with change tracking |
| src/AudiobookGenerator.Console/Menus/*.cs | New interactive menu components for chapters, images, metadata, voice selection, audio preview, and conversion |
| src/AudiobookGenerator.Console/Resources/Strings.* | New localized strings resource file with 200+ UI strings |
| src/AudiobookGenerator.Console/YewCone.AudiobookGenerator.Console.csproj | Added Spectre.Console package and resource file compilation |
| global.json | Updated SDK version to 10.0.103 |
| Directory.Build.props | Updated target framework to net10.0-windows |
| Directory.Packages.props | Added Spectre.Console package version |
| YewCone.AudiobookGenerator.slnx | Added global.json to solution items, reordered projects |
Files not reviewed (1)
- src/AudiobookGenerator.Console/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (7)
src/AudiobookGenerator.Console/Models/BookEditSession.cs:114
- The IsCoverImage method compares only the first 200 bytes of images, which could produce false positives if multiple images have identical headers. Consider comparing the full byte array or using a more robust comparison like a hash. If performance is a concern, at least compare the full length first before comparing content:
CoverImage != null && image.Content.Length == CoverImage.Length && image.Content.SequenceEqual(CoverImage)
This issue also appears on line 124 of the same file.
public bool IsCoverImage(BookImage image) =>
CoverImage != null && image.Content.Take(200).SequenceEqual(CoverImage.Take(200));
src/AudiobookGenerator.Console/Models/BookEditSession.cs:125
- The HasEdits property uses ReferenceEquals to check if the cover image changed, but SetCoverImage always creates a new reference by assigning
image.Content. This means HasEdits will always be true if SetCoverImage was called, even if the same image was selected. Consider comparing by content or using a different tracking mechanism for cover image changes.
|| !ReferenceEquals(CoverImage, _originalBook.CoverImage)
|| _images.Count != _originalBook.Images.Length;
src/AudiobookGenerator.Core/BookConverter.cs:392
- Making BookConverter a sealed class prevents inheritance. While this is often good practice for classes not designed for inheritance, verify that this doesn't break any existing usage patterns or extension points in the codebase. If this is intentional to prevent misuse, consider documenting the reasoning.
This issue also appears on line 398 of the same file.
public sealed class BookConverter(
src/AudiobookGenerator.Core/BookConverter.cs:400
- Exposing the Parser and Synthesizer properties publicly changes the API surface of BookConverter. These properties allow direct access to dependencies that were previously encapsulated. Consider whether this is the intended design or if these should be internal. If they must be public, ensure they're documented with XML comments explaining their purpose and intended usage patterns.
public IEpubBookParser Parser { get; } = bookParser;
public IAudioSynthesizer Synthesizer { get; } = synthesizer;
src/AudiobookGenerator.Console/Menus/ImageManager.cs:125
- Using
int.Parsewithout try-catch could throw FormatException if the choice string format is unexpected. While the choices are generated by the code itself (line 109), it's still safer to useint.TryParseor wrap in try-catch to handle potential edge cases gracefully. The same issue exists in similar code at lines 199, 45 (ChapterEditor.cs), and 72 (AudioPreviewer.cs).
var index = int.Parse(choice.Split('.')[0]) - 1;
src/AudiobookGenerator.Console/Program.cs:347
- Same issue as in BookEditSession.IsCoverImage: comparing only the first 200 bytes of images could produce false positives if multiple images have identical headers. Use the same fix suggested for BookEditSession.cs line 113-114.
book.CoverImage != null && i.Content.Take(200).SequenceEqual(book.CoverImage.Take(200))
? $"{Markup.Escape(i.FileName)} [yellow]{Strings.LabelCover}[/]"
src/AudiobookGenerator.Wpf/ViewModels/AudiobookGeneratorViewModel.cs:112
- The parameter name
loggerInstanceis unnecessarily verbose. The standard convention is to useloggeras the parameter name, especially since the field is namedlogger. This would allow direct assignment without renaming:this.logger = loggeror simplylogger = loggerwith the field assignment in the constructor body.
ILogger<AudiobookGeneratorViewModel> loggerInstance)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add console version with UI created by Copilot