Skip to content

feat (Editor): Add an option to set the editor font family#1078

Merged
GT-610 merged 2 commits intolollipopkit:mainfrom
GT-610:editor-font
Mar 20, 2026
Merged

feat (Editor): Add an option to set the editor font family#1078
GT-610 merged 2 commits intolollipopkit:mainfrom
GT-610:editor-font

Conversation

@GT-610
Copy link
Collaborator

@GT-610 GT-610 commented Mar 20, 2026

Resolve #1022.

Add the editorFontFamily property to the settings storage and implement font family selection functionality on editor-related pages. Also, update all pages that use the editor settings to support the newly added font family option.

Switch fl_lib to before-sqlite as main branch contains incomplete SQLite migration

Summary by CodeRabbit

  • New Features

    • Editor font family customization: choose and save a custom font for the editor via a new settings dialog; the chosen font is applied when editing files.
  • Chores

    • Updated submodule configuration for an internal library.
    • Added a new contributor to the participants list.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b99c78d4-e59c-4132-a072-65a5213d22ea

📥 Commits

Reviewing files that changed from the base of the PR and between a39ecf8 and 01a54f9.

📒 Files selected for processing (3)
  • lib/view/page/setting/entries/app.dart
  • lib/view/page/storage/local.dart
  • lib/view/page/storage/sftp.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/view/page/storage/local.dart

📝 Walkthrough

Walkthrough

Updated settings and UI to add a persistent editorFontFamily setting and propagate it into editor navigation arguments. Added a settings UI row and a dialog to edit the font-family string, persisted via the settings store, and ensured selected fonts are loaded where relevant. Switched several call sites to read settings from Stores.setting instead of SettingStore.instance. Updated .gitmodules to change the fl_lib submodule tracked branch to before-sqlite and advanced the packages/fl_lib submodule pointer.

Possibly related PRs

  • lollipopkit/flutter_server_box PR 1026: Modifies the settings store and settings-page files (same files touched here), indicating closely related changes to settings management.
  • lollipopkit/flutter_server_box PR 1046: Adds/adjusts the fl_lib submodule entry in .gitmodules, directly related to the submodule configuration change.
  • lollipopkit/flutter_server_box PR 1052: Advances the packages/fl_lib submodule pointer, related to the submodule commit update included here.

Suggested reviewers

  • lollipopkit
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat (Editor): Add an option to set the editor font family' clearly and specifically describes the main change: adding an option to configure the editor font family.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #1022 by adding editorFontFamily setting, implementing font-family selection dialogs across editor pages, and making the option optional and configurable.
Out of Scope Changes check ✅ Passed The fl_lib submodule branch change to before-sqlite is noted in the PR description as necessary due to incomplete SQLite migration in main, providing rationale. All other changes directly support the font family feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot requested a review from lollipopkit March 20, 2026 04:29
Add the `editorFontFamily` property to the settings storage and implement font family selection functionality on editor-related pages. Also, update all pages that use the editor settings to support the newly added font family option.
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/view/page/storage/sftp.dart (1)

20-20: ⚠️ Potential issue | 🟡 Minor

Remove unused import flagged by pipeline.

The pipeline reports this import as unused. Since all settings now use Stores.setting (from line 19), this import is no longer needed.

🧹 Proposed fix
-import 'package:server_box/data/store/setting.dart';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/storage/sftp.dart` at line 20, Remove the unused import of server_box/data/store/setting.dart from lib/view/page/storage/sftp.dart; since the code uses Stores.setting (imported on line 19) and no symbols from Setting are referenced, delete the import statement to satisfy the pipeline and avoid unused-import warnings. 
🧹 Nitpick comments (4)
.gitmodules (1)

12-12: The before-sqlite branch exists and can be used.

The branch has been verified to exist in the fl_lib repository. Since the PR description already documents the reason for this dependency switch (avoiding an incomplete SQLite migration on main), consider tracking when the main branch is fixed so this can be reverted to the stable branch in the future.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitmodules at line 12, The .gitmodules entry currently pins the fl_lib submodule to branch = before-sqlite; keep this change but add a short TODO comment in the same .gitmodules (or adjacent repo docs) noting why we pin to before-sqlite and a clear follow-up action to revert to the stable/main branch when the SQLite migration is completed (include the PR/issue number or a tracking label); ensure the note references the .gitmodules branch line so reviewers can find it easily and create a follow-up ticket if one does not exist. 
lib/view/page/storage/sftp.dart (1)

363-369: Redundant fetch of editorFontFamily.

The value is fetched twice - once for the .isEmpty check and again for the value. Store in a local variable for efficiency.

♻️ Proposed fix
+ final fontFamily = Stores.setting.editorFontFamily.fetch(); await EditorPage.route.go( context, args: EditorPageArgs( path: localPath, onSave: (_) async { // ... onSave implementation }, closeAfterSave: Stores.setting.closeAfterSave.fetch(), softWrap: Stores.setting.editorSoftWrap.fetch(), enableHighlight: Stores.setting.editorHighlight.fetch(), - fontFamily: Stores.setting.editorFontFamily.fetch().isEmpty - ? null - : Stores.setting.editorFontFamily.fetch(), + fontFamily: fontFamily.isEmpty ? null : fontFamily, ), );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/storage/sftp.dart` around lines 363 - 369, The code redundantly calls Stores.setting.editorFontFamily.fetch() twice when building the EditorConfig; fetch the value once into a local variable (e.g., var fontFamily = Stores.setting.editorFontFamily.fetch()), then use fontFamily.isEmpty ? null : fontFamily for the fontFamily field. Update the surrounding builder (where closeAfterSave, softWrap, enableHighlight are set) to reference that local variable instead of calling fetch() twice. 
lib/view/page/setting/entries/app.dart (1)

475-481: Implementation is correct; minor efficiency improvement possible.

The settings are consistently accessed via _setting. Same pattern of double-fetching editorFontFamily - consider storing in a local variable.

♻️ Proposed fix
+ final fontFamily = _setting.editorFontFamily.fetch(); await EditorPage.route.go( context, args: EditorPageArgs( text: text, lang: ProgLang.json, title: libL10n.setting, onSave: onSave, closeAfterSave: _setting.closeAfterSave.fetch(), softWrap: _setting.editorSoftWrap.fetch(), enableHighlight: _setting.editorHighlight.fetch(), - fontFamily: _setting.editorFontFamily.fetch().isEmpty - ? null - : _setting.editorFontFamily.fetch(), + fontFamily: fontFamily.isEmpty ? null : fontFamily, ), );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/setting/entries/app.dart` around lines 475 - 481, The code double-fetches _setting.editorFontFamily when computing fontFamily; cache it in a local variable (e.g., final fam = _setting.editorFontFamily.fetch()) before the widget build and use that variable in the fontFamily expression to avoid redundant fetch() calls and improve efficiency; keep the other _setting.fetch() calls as-is. 
lib/view/page/storage/local.dart (1)

370-376: Inconsistent store access and redundant fetch.

  1. Lines 370-372 use SettingStore.instance while lines 373-375 use Stores.setting for accessing the same store. For consistency within this code block, use one or the other.

  2. Stores.setting.editorFontFamily.fetch() is called twice - once for the .isEmpty check and once for the value. Consider storing the result in a local variable.

♻️ Proposed fix
+ final setting = SettingStore.instance; + final fontFamily = setting.editorFontFamily.fetch(); await EditorPage.route.go( context, args: EditorPageArgs( path: file.absolute.path, onSave: (_) { context.showSnackBar(libL10n.saved); setStateSafe(() {}); }, - closeAfterSave: SettingStore.instance.closeAfterSave.fetch(), - softWrap: SettingStore.instance.editorSoftWrap.fetch(), - enableHighlight: SettingStore.instance.editorHighlight.fetch(), - fontFamily: Stores.setting.editorFontFamily.fetch().isEmpty - ? null - : Stores.setting.editorFontFamily.fetch(), + closeAfterSave: setting.closeAfterSave.fetch(), + softWrap: setting.editorSoftWrap.fetch(), + enableHighlight: setting.editorHighlight.fetch(), + fontFamily: fontFamily.isEmpty ? null : fontFamily, ), );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/storage/local.dart` around lines 370 - 376, Use a consistent store accessor and avoid calling editorFontFamily.fetch() twice: choose either SettingStore.instance or Stores.setting and use it for all four properties (closeAfterSave, editorSoftWrap/editorSoftWrap.fetch for softWrap, editorHighlight for enableHighlight, and editorFontFamily). Cache the result of editorFontFamily.fetch() in a local variable (e.g., final editorFont = <storeAccessor>.editorFontFamily.fetch()) and then set fontFamily to null if editorFont.isEmpty else editorFont; update the references to the chosen store accessor throughout this block. 
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Outside diff comments: In `@lib/view/page/storage/sftp.dart`: - Line 20: Remove the unused import of server_box/data/store/setting.dart from lib/view/page/storage/sftp.dart; since the code uses Stores.setting (imported on line 19) and no symbols from Setting are referenced, delete the import statement to satisfy the pipeline and avoid unused-import warnings. --- Nitpick comments: In @.gitmodules: - Line 12: The .gitmodules entry currently pins the fl_lib submodule to branch = before-sqlite; keep this change but add a short TODO comment in the same .gitmodules (or adjacent repo docs) noting why we pin to before-sqlite and a clear follow-up action to revert to the stable/main branch when the SQLite migration is completed (include the PR/issue number or a tracking label); ensure the note references the .gitmodules branch line so reviewers can find it easily and create a follow-up ticket if one does not exist. In `@lib/view/page/setting/entries/app.dart`: - Around line 475-481: The code double-fetches _setting.editorFontFamily when computing fontFamily; cache it in a local variable (e.g., final fam = _setting.editorFontFamily.fetch()) before the widget build and use that variable in the fontFamily expression to avoid redundant fetch() calls and improve efficiency; keep the other _setting.fetch() calls as-is. In `@lib/view/page/storage/local.dart`: - Around line 370-376: Use a consistent store accessor and avoid calling editorFontFamily.fetch() twice: choose either SettingStore.instance or Stores.setting and use it for all four properties (closeAfterSave, editorSoftWrap/editorSoftWrap.fetch for softWrap, editorHighlight for enableHighlight, and editorFontFamily). Cache the result of editorFontFamily.fetch() in a local variable (e.g., final editorFont = <storeAccessor>.editorFontFamily.fetch()) and then set fontFamily to null if editorFont.isEmpty else editorFont; update the references to the chosen store accessor throughout this block. In `@lib/view/page/storage/sftp.dart`: - Around line 363-369: The code redundantly calls Stores.setting.editorFontFamily.fetch() twice when building the EditorConfig; fetch the value once into a local variable (e.g., var fontFamily = Stores.setting.editorFontFamily.fetch()), then use fontFamily.isEmpty ? null : fontFamily for the fontFamily field. Update the surrounding builder (where closeAfterSave, softWrap, enableHighlight are set) to reference that local variable instead of calling fetch() twice. 

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: df18508a-04d6-469a-a053-7acaadee783a

📥 Commits

Reviewing files that changed from the base of the PR and between 728116e and a39ecf8.

📒 Files selected for processing (9)
  • .gitmodules
  • lib/data/res/github_id.dart
  • lib/data/store/setting.dart
  • lib/view/page/setting/entries/app.dart
  • lib/view/page/setting/entries/editor.dart
  • lib/view/page/setting/entries/ssh.dart
  • lib/view/page/storage/local.dart
  • lib/view/page/storage/sftp.dart
  • packages/fl_lib
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 20, 2026
Standardize the method for retrieving font settings on the editor page to avoid redundant calls to the `fetch()` method. Extract the font retrieval logic into variables or anonymous functions to improve code readability and performance.
@GT-610 GT-610 merged commit f858b15 into lollipopkit:main Mar 20, 2026
2 checks passed
@GT-610 GT-610 deleted the editor-font branch March 20, 2026 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant