New code save #95
Merged
New code save #95
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch introduces a feature to save and restore the history of code written in the editor. Key features: - Code history is automatically saved when you stop typing for 2 seconds. - A new screen is added to view the list of saved histories, sorted by timestamp. - You can select a history entry to view the full code and restore it to the editor. - The history is stored locally on your device, with each problem having its own history. Note on tests: The existing test suite has some failing tests (`widget_test.dart` and `update_service_test.dart`). These failures appear to be caused by pre-existing issues in the test setup, specifically related to network requests in the test environment. I have made an effort to fix the `widget_test.dart` by providing the necessary providers, but the underlying issue with async operations during app startup in tests remains. These failing tests are unrelated to the new code history feature.
…en.dart and widget_test.dart; adopt dev styles and test setup
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋レビュー概要
このプルリクエストは新しいコード履歴機能を導入し、ユーザーが問題ごとにコードを自動的に保存・復元できるようにします。実装には、新しいデータモデル、ファイルベースの履歴保存のためのサービス、履歴を表示するための新しい画面が含まれます。エディタ画面も大幅にリファクタリングされ、新しいUIレイアウトと自動保存機能が追加された。
全体として、これはユーザー体験を大幅に改善する素晴らしい機能追加である。コードは全般的によく構造化されている。私のレビューは、堅牢性、保守性、テストの改善に重点を置いています。
ᔍ 一般的なフィードバック
- エラー処理**： 新しいファイルベースサービスには、I/O例外によるクラッシュを防ぐためのエラーハンドリングを含めるべきです。
- CI/CD**： GitHub Actions ワークフローには、正しく実行するために対処すべき設定上の問題があります。
- テスト： 新機能は、その正しさを保証し、将来のリグレッションを防ぐために、専用のウィジェットテストが役立つでしょう。
- コードのクリーン化**： 削除されるべきデバッグログが残っています。
エディター画面のUI改善は素晴らしく、インターフェイスをより直感的にしています。よくやった！
*** Translated with www.DeepL.com/Translator (free version) ***
| - '.github/workflows/build.yml' | ||
| # このファイル名に合わせてパスを更新 | ||
| - '.github/workflows/flutter-ci.yml' | ||
| - 'android/**' |
There was a problem hiding this comment.
- 🔴 critical: The trigger path for this workflow file is incorrect. The file is named
build.yaml, but the path in the trigger is set to.github/workflows/flutter-ci.yml. This will prevent the workflow from running when this file itself is changed.
Suggested change
| - 'android/**' | |
| - '.github/workflows/build.yaml' |
| steps: | ||
| # Step 1: リポジトリのコードをチェックアウト | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
- 🟡 medium: The
containsfunction is not necessary here and makes the condition harder to read. A direct string comparison is simpler and achieves the same result. This comment applies to the otherifconditions in this job as well.
Suggested change
| uses: actions/checkout@v4 | |
| if: (github.event.inputs.build_type || 'split') == 'split' |
| # <--- 変更点: 以下の2つのステップでアップロードするファイルを分岐 --- > | ||
|
|
||
| # 分割APKをアップロード | ||
| - name: Upload Split APKs Artifact |
There was a problem hiding this comment.
- 🟡 medium: The
containsfunction is not necessary here. A direct string comparison is simpler and more readable. This comment also applies to theRelease with Single APKstep.
Suggested change
| - name: Upload Split APKs Artifact | |
| if: needs.build_android.outputs.build_type == 'split' |
| // CodeController の言語も更新 | ||
| _codeController.language = _getHighlightLanguage(_selectedLanguage); | ||
| developer.log('Language changed to: $_selectedLanguage, loading code...', name: 'EditorScreen'); | ||
| developer.log( |
There was a problem hiding this comment.
- 🟢 low: This file contains a large number of
developer.logandprintstatements. While useful for debugging, they should be removed or replaced with a more structured logging solution before this feature is released to production to avoid cluttering the console logs.
Suggested change
| developer.log( | |
| // developer.log('Language changed to: $_selectedLanguage, loading code...', name: 'EditorScreen'); |
| import '../services/code_history_service.dart'; | ||
| import 'code_history_screen.dart'; | ||
|
|
||
| // 3点メニュー用のアクション列挙体（トップレベルに定義） |
There was a problem hiding this comment.
- 🟢 low: For better encapsulation, you could define this enum inside the
_EditorScreenStateclass, as it's only used there. However, since it's a private enum (_ToolbarAction), its scope is already limited to this file, so this is a minor stylistic suggestion.
Suggested change
| // 3点メニュー用のアクション列挙体（トップレベルに定義） | |
| enum _ToolbarAction { save, history, restore, reset, share } | |
| class _EditorScreenState extends State<EditorScreen> { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.