Repository navigation
Add persistent line number toggle for TXT previews - #2037
MORTEM3845 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Line-number state and menu toggle QuickLook.Plugin/QuickLook.Plugin.TextViewer/TextViewerPanel.cs, QuickLook.Plugin/QuickLook.Plugin.TextViewer/Plugin.MoreMenu.cs |
The text viewer loads line-number visibility from settings and updates padding when the state changes. The TXT preview menu displays a checked item when line numbers are enabled and saves changes through the toggle. |
Option documentation and localized menu label OPTIONS.md, QuickLook.Plugin/QuickLook.Plugin.TextViewer/Translations.config |
The options documentation describes the Boolean setting and its default. Translation entries add the menu label across supported locales. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Feature · Severity of issue fixed: Low
Sequence Diagram(s)
sequenceDiagram
participant User
participant MoreMenu as Plugin.MoreMenu
participant Panel as TextViewerPanel
participant Settings as Text viewer settings
User->>MoreMenu: Select Show Line Numbers
MoreMenu->>Panel: ToggleLineNumbers()
Panel->>Panel: Update padding and separator
Panel->>Settings: Save ShowLineNumbers
MoreMenu->>MoreMenu: Update checkmark
Suggested reviewers: emako
Merge Risk: 
a8546
-
TXT previews restore and save the line-number choice, and stale menu commands do not affect a replaced viewer. No actionable merge risk remains.
Security architecture risk: 75418
-
The new action changes only line-number presentation and saves a fixed Boolean preference. The reviewed change does not introduce broader file access, elevated privileges or a new security boundary.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The newly reachable operation affects the captured viewer's presentation and the shared TextViewer preference in its existing configuration domain. The command does not accept a caller-selected configuration path, key or payload.
Trust Boundaries and Controls
- inferred — A selected file path can determine whether the TXT menu item appears, but it is not propagated into the new setting write. Execution invokes only the fixed presentation toggle, so this path does not establish an attacker-controlled authority expansion.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: a persistent line-number toggle for TXT previews. Linked Issues check Issue #932asks for an option to remove line numbers from TXT previews. Direct evidence at head a8546a8: TextViewerPanel.cs reads ShowLineNumbers from SettingHelper.Get("ShowLineNumbers", true, "Quick…Out of Scope Changes check All changed areas connect to the TXT line-number objective. TextViewerPanel.cs implements the setting, default, inset, and toggle. Plugin.MoreMenu.cs adds the menu entry and its stale-command guard. T… Full details: Linked Issues check
-
Explanation
Issue
#932asks for an option to remove line numbers from TXT previews. Direct evidence at head a8546a8: TextViewerPanel.cs reads ShowLineNumbers from SettingHelper.Get("ShowLineNumbers", true, "QuickLook.Plugin.TextViewer"), so line numbers stay enabled by default. The new public ToggleLineNumbers() flips ShowLineNumbers, persists it with SettingHelper.Set, and hides the AvalonEdit separator. UpdateTextPadding() applies a 6-DIP left inset only when line numbers are hidden. Plugin.MoreMenu.cs adds a TXT-only Editor_ShowLineNumbers item with a check mark that reflects ShowLineNumbers. The menu handler ignores stale commands when _tvp no longer references the captured viewer. The toggle, default, persistence, and inset requirements are implemented in code. The RTF bullet in#932is a secondary request. The PR states RTF rendering already works and is unchanged. This review did not re-verify RTF behavior. The reported WPF integration checks were not independently re-run. The changes since the previous review (stale-command guard and separator refactor) keep the earlier conclusion valid.
Full details: Out of Scope Changes check
-
Explanation
All changed areas connect to the TXT line-number objective. TextViewerPanel.cs implements the setting, default, inset, and toggle. Plugin.MoreMenu.cs adds the menu entry and its stale-command guard. The OPTIONS.md documentation covers the new ShowLineNumbers setting. Translations.config adds the Editor_ShowLineNumbers label. The same file also adds locale blocks for the new languages with translations of existing strings. These support the menu label and are not product behavior changes. The constructor separator lookup was reformatted to one line. It is the same logic that ToggleLineNumbers reuses, so it is not an unrelated change. No unrelated product behavior was identified. RTF rendering is unchanged, as the PR states. Note: the raw summary lists 19 existing and 9 new locale blocks (28 total), while the PR text says 29 locales. This count was not verified against the file and does not affect the scope verdict.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
PR Checklist
Brief Description of Changes
TXT previews always display line numbers. Add Show line numbers to the preview's … menu, with a check mark reflecting the current state.
Save the shared TextViewer preference through the existing
SettingHelpermechanism, keeping line numbers enabled by default. When hidden, retain a 6-DIP left inset for readability. Document the setting and translate the new menu label into all 29 supported plugin locales.Related Issue (if any)
Fixes #932. RTF rendering is already supported and is unchanged.
Additional Notes
Validation:
git diff --checkpassed.Line numbers enabled
Line numbers hidden
Summary by CodeRabbit
ShowLineNumberssetting, including its default value and an example of disabling line numbers. The documentation also explains that the menu choice is saved.