Conversation
rami123
changed the title
📝 WalkthroughWalkthroughFerrite-Ramix extends Ferrite with Rendered-view navigation improvements, chapters and link interactions, configurable tab and viewer behavior, dynamic font sizing, portable Windows builds, and comprehensive documentation of fork-specific features. ChangesViewer features and application integration
Portable packaging and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MarkdownEditor
participant CentralPanel
participant Navigation
participant AppState
User->>MarkdownEditor: Click local Markdown link
MarkdownEditor->>CentralPanel: Report local_file_clicked
CentralPanel->>Navigation: Record parent→child relationship
Navigation->>AppState: Open file in new tab
AppState-->>CentralPanel: Activate opened tab
User->>CentralPanel: Click "← Back" or Alt+Left
CentralPanel->>AppState: Restore parent tab
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/editor/ferrite/editor.rs (1)
400-402: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared font-size bounds.
This change required manually synchronizing literals with
Settings; reference its constants in both implementation and test to prevent future drift.Proposed refactor
- let new_size = size.clamp(6.0, 72.0); + let new_size = size.clamp( + crate::config::Settings::MIN_FONT_SIZE, + crate::config::Settings::MAX_FONT_SIZE, + );- assert!((editor.font_size() - 6.0).abs() < 0.01); + assert!( + (editor.font_size() - crate::config::Settings::MIN_FONT_SIZE).abs() < 0.01 + );Also applies to: 3821-3821
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/ferrite/editor.rs` around lines 400 - 402, Update set_font_size to use the shared minimum and maximum font-size constants from Settings instead of hardcoded bounds. Update the related font-size test to reference those same Settings constants, preserving the existing clamping behavior and preventing future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/central_panel.rs`:
- Around line 2852-2883: Update cleanup_tab_state to remove closed-tab entries
from both newly introduced per-tab maps: remove the tab ID from
self.state.ui.navigation_parents in src/app/central_panel.rs:2852-2883 and from
self.rendered_chapter_tracking in src/app/navigation.rs:755-755. Preserve the
existing cleanup behavior for all other tab state.
- Around line 791-800: Update the back-navigation input handling around alt_left
and ctrl_backspace so linked-tab navigation does not consume editor shortcuts
while a text editor is focused. Gate Ctrl+Backspace and Alt+Left on the
appropriate non-editor-focus condition, while preserving navigation behavior
when focus is outside an editor and retaining mouse_back handling.
In `@src/app/dialogs.rs`:
- Around line 58-64: Update the Save action handling in the dialog to explicitly
handle PendingAction::CloseOtherTabs and PendingAction::CloseAllTabs by
performing the bulk save-and-confirm flow, or disable Save for those pending
actions. Ensure pending_action is cleared and the dialog closes instead of
remaining open.
In `@src/app/status_bar.rs`:
- Around line 560-564: Update the font-size wording in src/app/status_bar.rs
lines 560-564 and src/ui/settings.rs lines 1503-1507 to use logical points
(“pt”) or omit the unit, keeping the status label and tooltip terminology
consistent.
In `@src/editor/outline.rs`:
- Around line 728-773: Update test_parse_image assertions to match parse_image’s
Option<(String, String)> return type, checking both the parsed title and
filename in each expected tuple while preserving the existing test cases.
In `@src/ui/settings.rs`:
- Around line 1110-1117: Move the “Always start maximized” control out of the
Rendered-mode UI settings section and into a general window settings section,
keeping its behavior unchanged. Update the surrounding grouping so the
Rendered-mode description only covers settings that affect Rendered mode.
---
Nitpick comments:
In `@src/editor/ferrite/editor.rs`:
- Around line 400-402: Update set_font_size to use the shared minimum and
maximum font-size constants from Settings instead of hardcoded bounds. Update
the related font-size test to reference those same Settings constants,
preserving the existing clamping behavior and preventing future drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d91cad1-7960-47b5-a90f-368ace2b2dc1
⛔ Files ignored due to path filters (5)
Ramix - Readme/Bug - Ferrite wrongly estimated rendered positions using source line × plain row height causing mismatch bwtween main view, and outline/Cap_0040.pngis excluded by!**/*.pngRamix - Readme/Bug - Ferrite wrongly estimated rendered positions using source line × plain row height causing mismatch bwtween main view, and outline/Cap_0041.pngis excluded by!**/*.pngRamix - Readme/Screenshots/Cap_0144.pngis excluded by!**/*.pngRamix - Readme/Screenshots/Cap_0161.pngis excluded by!**/*.pngportable/FerriteMDPortable/FerriteMDPortable.exeis excluded by!**/*.exe
📒 Files selected for processing (46)
.gitignoreREADME.mdRamix - Readme/Bug - Ferrite wrongly estimated rendered positions using source line × plain row height causing mismatch bwtween main view, and outline/____lnk-url_README.mdRamix - Readme/README.mdbuild-portable-fast.batportable/.gitignoreportable/FerriteMDPortable/App/AppInfo/Launcher/FerriteMDPortable.iniportable/FerriteMDPortable/App/AppInfo/appinfo.iniportable/FerriteMDPortable/App/DefaultData/settings/.gitkeepportable/FerriteMDPortable/Data/settings/.gitkeepportable/FerriteMDPortable/Other/Help/Images/.gitkeepportable/FerriteMDPortable/Other/Help/help.htmlportable/FerriteMDPortable/Other/Source/LICENSEportable/FerriteMDPortable/help.htmlportable/README.mdportable/installer.nsiscripts/build-portable-custom.ps1src/app/central_panel.rssrc/app/dialogs.rssrc/app/keyboard.rssrc/app/mod.rssrc/app/navigation.rssrc/app/status_bar.rssrc/config/session.rssrc/config/settings.rssrc/editor/ferrite/editor.rssrc/editor/outline.rssrc/editor/widget.rssrc/main.rssrc/markdown/code_execution.rssrc/markdown/csv_viewer.rssrc/markdown/editor.rssrc/markdown/mod.rssrc/markdown/parser.rssrc/markdown/rendered_session.rssrc/markdown/video_embed.rssrc/markdown/widgets.rssrc/preview/sync_scroll.rssrc/state.rssrc/terminal/pty.rssrc/ui/about.rssrc/ui/outline_panel.rssrc/ui/productivity_panel.rssrc/ui/ribbon.rssrc/ui/settings.rssrc/ui/terminal_panel.rs
💤 Files with no reviewable changes (9)
- portable/FerriteMDPortable/Other/Source/LICENSE
- portable/FerriteMDPortable/App/AppInfo/appinfo.ini
- portable/.gitignore
- portable/FerriteMDPortable/App/AppInfo/Launcher/FerriteMDPortable.ini
- portable/FerriteMDPortable/Other/Help/help.html
- portable/README.md
- portable/FerriteMDPortable/help.html
- portable/installer.nsi
- src/ui/about.rs
Comment on lines
+2852
to
+2883
| // Open local Markdown links only after editor rendering releases its tab borrow. | ||
| if let Some((path, parent_tab_id)) = pending_local_file { | ||
| let time = self.get_app_time(); | ||
| match self.open_file_smart(path.clone(), true, Some(time)) { | ||
| Ok(index) => { | ||
| let child_tab_id = self.state.tab(index).map(|tab| tab.id); | ||
| if let Some(child_tab_id) = child_tab_id { | ||
| if child_tab_id != parent_tab_id { | ||
| self.state | ||
| .ui | ||
| .navigation_parents | ||
| .insert(child_tab_id, parent_tab_id); | ||
| } | ||
| } | ||
| if let Some(tab) = self.state.tab_mut(index) { | ||
| if let TabKind::ImageViewer(viewer) = &mut tab.kind { | ||
| viewer.parent_tab_id = Some(parent_tab_id); | ||
| } | ||
| } | ||
| } | ||
| Err(error) => { | ||
| log::warn!( | ||
| "Failed to open local Markdown link '{}': {}", | ||
| path.display(), | ||
| error | ||
| ); | ||
| self.state | ||
| .show_error(format!("Failed to open file:\n{}", error)); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
New per-tab maps aren't pruned on tab close. cleanup_tab_state intentionally removes every other per-tab map (tree_viewer_states, csv_viewer_states, sync_scroll_states) to prevent leaks, but the two maps added in this PR keep accumulating entries for closed tabs for the whole session. Not a correctness bug (tab ids aren't reused), but it drifts from the established cleanup contract.
src/app/central_panel.rs#L2852-L2883: entries inserted intoself.state.ui.navigation_parentshere are never removed; addself.state.ui.navigation_parents.remove(&tab_id)incleanup_tab_state.src/app/navigation.rs#L755-L755: entries created inrendered_chapter_trackinghere are never removed; addself.rendered_chapter_tracking.remove(&tab_id)incleanup_tab_state.
📍 Affects 2 files
src/app/central_panel.rs#L2852-L2883(this comment)src/app/navigation.rs#L755-L755
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/central_panel.rs` around lines 2852 - 2883, Update cleanup_tab_state
to remove closed-tab entries from both newly introduced per-tab maps: remove the
tab ID from self.state.ui.navigation_parents in
src/app/central_panel.rs:2852-2883 and from self.rendered_chapter_tracking in
src/app/navigation.rs:755-755. Preserve the existing cleanup behavior for all
other tab state.
Comment on lines
+560
to
+564
| ui.separator(); | ||
| ui.label(format!( | ||
| "Font size: {:.0} px", | ||
| self.current_editor_font_size(&ctx) | ||
| )); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Label logical font sizes consistently as points, not pixels.
Both new strings describe egui logical point sizes as physical pixels, which is inaccurate under DPI scaling.
src/app/status_bar.rs#L560-L564: useptor omit the unit in the status label.src/ui/settings.rs#L1503-L1507: use the same terminology in the tooltip.
📍 Affects 2 files
src/app/status_bar.rs#L560-L564(this comment)src/ui/settings.rs#L1503-L1507
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/status_bar.rs` around lines 560 - 564, Update the font-size wording
in src/app/status_bar.rs lines 560-564 and src/ui/settings.rs lines 1503-1507 to
use logical points (“pt”) or omit the unit, keeping the status label and tooltip
terminology consistent.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/dialogs.rs`:
- Around line 125-130: Localize the bulk-action labels in the dialog button
logic, including the save-label branch near the visible code and the
corresponding discard-label branch. Add translation keys for “Save all” and
“Discard all” to the appropriate localization resources, then replace the
hardcoded strings with t!(...) calls consistent with the adjacent button labels.
In `@src/state.rs`:
- Around line 4380-4395: Bound the unsaved-title lists used by the Close Other
Tabs flow at src/state.rs lines 4380-4395, Close All Tabs flow at lines
4427-4439, and exit prompt at lines 5651-5666 by displaying only a limited
number of titles with a remaining-count indicator, or by rendering each list in
a bounded ScrollArea; preserve the existing confirmation behavior and formatting
for the displayed titles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 780acfcd-419c-4b8e-9960-7c3b4fc8e982
⛔ Files ignored due to path filters (2)
Ramix - Readme/Screenshots/Cap_0161.pngis excluded by!**/*.pngRamix - Readme/Screenshots/Cap_0251.pngis excluded by!**/*.png
📒 Files selected for processing (11)
README.mdRamix - Readme/README.mdsrc/app/central_panel.rssrc/app/dialogs.rssrc/app/mod.rssrc/app/status_bar.rssrc/config/settings.rssrc/editor/ferrite/editor.rssrc/editor/outline.rssrc/state.rssrc/ui/settings.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- src/app/status_bar.rs
- Ramix - Readme/README.md
- src/ui/settings.rs
- README.md
- src/config/settings.rs
- src/app/mod.rs
- src/app/central_panel.rs
- src/editor/outline.rs
Comment on lines
125
to
+130
| // "Save" button - save then proceed with action | ||
| if ui | ||
| .button(t!("dialog.unsaved_changes.save").to_string()) | ||
| .clicked() | ||
| { | ||
| let save_label = if is_bulk_close || is_exit { | ||
| "Save all".to_string() | ||
| } else { | ||
| t!("dialog.unsaved_changes.save").to_string() | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the bulk-action labels.
Save all and Discard all remain English under every configured UI language. Add translation keys and use t!(...), consistent with the adjacent buttons.
Also applies to: 217-222
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/dialogs.rs` around lines 125 - 130, Localize the bulk-action labels
in the dialog button logic, including the save-label branch near the visible
code and the corresponding discard-label branch. Add translation keys for “Save
all” and “Discard all” to the appropriate localization resources, then replace
the hardcoded strings with t!(...) calls consistent with the adjacent button
labels.
Comment on lines
+4380
to
+4395
| let unsaved_titles: Vec<String> = self | ||
| .tabs | ||
| .iter() | ||
| .filter(|tab| { | ||
| tab.id != keep_tab_id | ||
| && tab.should_prompt_to_save(&self.settings, SavePromptContext::TabClose) | ||
| }) | ||
| .map(Tab::persisted_session_display_title) | ||
| .collect(); | ||
|
|
||
| if !unsaved_titles.is_empty() { | ||
| self.ui.show_confirm_dialog = true; | ||
| self.ui.confirm_dialog_message = format!( | ||
| "The following tabs have unsaved changes:\n\n• {}", | ||
| unsaved_titles.join("\n• ") | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bound the tab list shown in confirmation dialogs.
The non-resizable dialog renders this message without a scroll area. With enough unsaved tabs, the action buttons can be pushed outside the usable viewport. Show a limited number of titles plus a remaining count, or render the list in a bounded ScrollArea.
src/state.rs#L4380-L4395: bound the Close Other Tabs title list.src/state.rs#L4427-L4439: apply the same bound to Close All Tabs.src/state.rs#L5651-L5666: apply the same bound to the exit prompt.
📍 Affects 1 file
src/state.rs#L4380-L4395(this comment)src/state.rs#L4427-L4439src/state.rs#L5651-L5666
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/state.rs` around lines 4380 - 4395, Bound the unsaved-title lists used by
the Close Other Tabs flow at src/state.rs lines 4380-4395, Close All Tabs flow
at lines 4427-4439, and exit prompt at lines 5651-5666 by displaying only a
limited number of titles with a remaining-count indicator, or by rendering each
list in a bounded ScrollArea; preserve the existing confirmation behavior and
formatting for the displayed titles.
Owner
|
very nice :) I will look into this and include it in the next release, currently on vacation, scheduled to mid or late August |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Hi great viewer/editor! This PR first fixes an existing Rendered-view navigation bug: clicking a heading in the Outline could scroll to the wrong location.
The original navigation estimated the rendered position from the Markdown source-line number multiplied by a plain row height. That estimate becomes inaccurate when a document contains wrapped paragraphs, large headings, code blocks, tables, images, or other content whose rendered height differs from its source representation.
The fix uses the measured source-line-to-rendered-position mapping produced by the actual rendered layout. Outline and Chapters navigation therefore lands on the selected heading instead of a preceding or nearby section.
After fixing that bug, I added several viewer-oriented options for people who frequently open Markdown files for quick reading and navigation. These additions are also useful during editing, but their main purpose is to make Ferrite a more practical document viewer.
Almost all new behavior is optional and disabled by default, preserving Ferrite's existing behavior unless the user explicitly enables an option.
Type of Change
Primary Bug Fix
Viewer-Oriented Changes
Chapters panel
#,##,###, and so on).===or---underline syntax.Rendered-view interface
Document tabs
Links, images, and local files
Font-size workflow
Multi-tab Exit and bulk-close safety
Ferrite 0.3.0 had a minor multi-tab Exit bug: after warning about unsaved changes, the Save action called the single-document save handler only once, so it saved only the active tab despite a source comment claiming all modified tabs were saved. The Close All path also had no bulk-save implementation.
This update lists every affected modified filename for Exit, Close All, and Close Other, with Save all, Discard all, and Cancel actions. Save all processes every affected tab, including Save As for untitled documents; cancellation or failure aborts the bulk close. Exit also persists recovery content for all modified tabs before presenting the decision dialog.
Portable Windows build
portabledirectory besideferrite.exekeeps configuration and session data with the application.cargo check.Screenshots
Rendered view with File Tree and Chapters panel
Added Appearance settings
Additional bug reproduction material is included under:
Ramix - Readme/Bug - Ferrite wrongly estimated rendered positions using source line × plain row height causing mismatch bwtween main view, and outline/Defaults and Compatibility
The new viewer-interface, Chapters, tab-management, font carry-forward, maximized-startup, inline-image, and Escape-to-exit options are disabled by default.
Existing users therefore retain Ferrite's original interaction model until they deliberately enable the additions in Settings → Appearance.
The corrected Outline navigation, status-bar font-size display, link tooltips, supported local-file opening, the on-screen Back button, and the mouse Back button work without additional configuration. Alt+Left and Ctrl+Backspace Back navigation are separate options and default to off because enabling them may change normal editing or application shortcut behavior.
Checklist
cargo fmtand it produces no changescargo clippyand it produces no warningscargo testand all tests passcargo build --releasesuccessfullyBreaking Changes
No intentional breaking changes.
The existing behavior remains the default for almost all newly added options.
Testing
cargo build --release.Additional Notes
The fork began specifically to correct the Rendered Outline navigation bug. The additional features were added afterward to support a fast, viewer-focused Markdown workflow.
The changes are deliberately configurable rather than forcing a different workflow on existing Ferrite users.
Summary by CodeRabbit