Skip to content

feat: add iTerm2 inline image support - #310

Merged
NSPC911 merged 3 commits into
masterfrom
it2iip
Jul 29, 2026
Merged

feat: add iTerm2 inline image support#310
NSPC911 merged 3 commits into
masterfrom
it2iip

Conversation

@NSPC911

@NSPC911 NSPC911 commented Jul 28, 2026

Copy link
Copy Markdown
Owner

This is not fully made from scratch, I used the renderer from lnqs/textual-image#87


by submitting this pull request, i agree that

  • i have run poe check to check for any style issues and fixed them
  • i have tested rovr (and also ran poe test if applicable) to make sure my changes do not break anything
  • cache, logs, dotfiles and/or others were not accidentally added to git's tracking history
  • my commits (or at least the pr title) follow the conventional commits format as much as possible

Summary by Sourcery

Add support for rendering image previews via the iTerm2 inline image protocol with auto-detection in compatible terminals.

New Features:

  • Introduce an ITerm2-specific image widget to render previews using the iTerm2 Inline Image Protocol.
  • Allow selecting the ITerm2 protocol explicitly in configuration and first-launch setup, including auto-selection when supported.

Enhancements:

  • Integrate the new ITerm2 image widget into the existing preview container so the appropriate image backend is chosen based on config and terminal capabilities.

Documentation:

  • Update image preview documentation to describe the new ITerm2 protocol option, its auto-detection behavior, and the updated configuration keys.

Tests:

  • Add tests for the iTerm2 image sequence generator and terminal support detection logic.

Summary by CodeRabbit

  • New Features

    • Added built-in iTerm2 inline image previews for iTerm2 and WezTerm terminals.
    • Added automatic protocol selection when supported.
    • Added iTerm2 as a selectable image protocol during first-run setup.
    • Added support for configuring TGP and iTerm2 image protocols.
  • Documentation

    • Updated image preview configuration examples and supported protocol lists.
    • Clarified automatic protocol detection and iTerm2 support.
  • Tests

    • Added coverage for image sequence generation and terminal capability detection.

@sourcery-ai

sourcery-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds a custom iTerm2 Inline Image Protocol widget and wires it into image preview selection, configuration schema, first-launch UI, and documentation, with auto-detection for iTerm2/WezTerm terminals and basic tests.

Sequence diagram for auto-selecting ITerm2Image in preview container

sequenceDiagram
    participant Config
    participant PreviewContainer
    participant ITerm2Image
    participant TextualImageWidget

    PreviewContainer->>Config: read interface.image_viewer.protocol
    Config-->>PreviewContainer: image_protocol
    alt image_protocol == ITerm2
        PreviewContainer->>PreviewContainer: image_widget = ITerm2Image
    else image_protocol is empty
        PreviewContainer->>ITerm2Image: iterm2_supported()
        ITerm2Image-->>PreviewContainer: bool
        alt iterm2_supported() is True
            PreviewContainer->>PreviewContainer: image_widget = ITerm2Image
        else iterm2_supported() is False
            PreviewContainer->>TextualImageWidget: lookup AutoImage
            TextualImageWidget-->>PreviewContainer: Image class
            PreviewContainer->>PreviewContainer: image_widget = Image class
        end
    else other explicit protocol
        PreviewContainer->>TextualImageWidget: lookup protocol + Image
        TextualImageWidget-->>PreviewContainer: Image class
        PreviewContainer->>PreviewContainer: image_widget = Image class
    end
    PreviewContainer->>PreviewContainer: NewImage = partial(image_widget, classes="image_preview")
Loading

File-Level Changes

Change Details Files
Introduce ITerm2 inline image widget with terminal capability detection and rendering via iTerm2 inline image escape sequences.
  • Add is_supported() helper that checks for a TTY and TERM_PROGRAM in ("iTerm2", "WezTerm").
  • Implement ITerm2Image widget composed of an internal _ImageImpl that scales, crops, and renders images using PixelData and iTerm2 inline image escape sequences.
  • Add simple caching of rendered image data keyed by image, crop region, content size, and terminal cell size.
  • Render the image by emitting Rich Segments that clear the cell area, move the cursor, output the escape sequence, then restore cursor position.
src/rovr/components/iterm2_image.py
Wire the ITerm2 image widget into the preview container so it can be selected via config or automatically when appropriate.
  • Read image_viewer.protocol from config into a local variable.
  • Select ITerm2Image when protocol is explicitly "ITerm2" or when protocol is unset and is_supported() reports iTerm2 capability; otherwise fall back to the textual_image widget mapped from the protocol name.
  • Use the chosen image_widget in the NewImage partial while preserving the existing CSS class configuration.
src/rovr/core/preview_container.py
Expose ITerm2 as a selectable protocol in first-launch configuration, including auto-selection logic.
  • Import ITerm2Image and is_supported into the first_launch module.
  • Update prot_to_timg mapping so "auto" uses ITerm2Image when supported, otherwise timg.Image.
  • Add explicit "iterm2" mapping to ITerm2Image and include a matching entry in the protocol schema/label mappings.
  • Extend the protocol selection dropdown to include an "iTerm2" option bound to value "iterm2".
src/rovr/first_launch.py
Update configuration schema and type hints to support the new ITerm2 protocol value.
  • Extend the _RovrConfigInterfaceImageViewerProtocol Literal with "ITerm2".
  • Add a dedicated _ROVRCONFIGINTERFACEIMAGEVIEWERPROTOCOL_ITERM2 constant with appropriate docstring in the stub file.
src/rovr/classes/config.pyi
src/rovr/config/schema.json
Refresh image preview documentation to reflect ITerm2 support and the new config path/keys.
  • Replace the note claiming iTerm2 inline images are unsupported with a description of the built-in ITerm2 widget and auto-selection behavior based on TERM_PROGRAM.
  • Update configuration examples to use the [interface.image_viewer] section and protocol key instead of the old [settings]/image_protocol structure.
  • Document ITerm2 as an allowed protocol value in the public docs and dev docs.
docs/src/content/docs/dev/features/image-previews.mdx
docs/src/content/docs/features/image-previews.mdx
Add tests for ITerm2 image sequence construction and terminal support detection.
  • Test that _build_sequence generates the exact expected iTerm2 inline image escape sequence for given dimensions and base64 payload.
  • Parametrize tests over TERM_PROGRAM values ("iTerm2", "WezTerm") to ensure is_supported() returns True only when stdout is a TTY.
  • Verify that is_supported() returns False when stdout is not a TTY even if TERM_PROGRAM indicates support.
tests/test_iterm2_image.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@NSPC911, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 748645ef-35fc-401b-9826-3bf0d5e920ea

📥 Commits

Reviewing files that changed from the base of the PR and between 40bfe53 and 9c3d11a.

📒 Files selected for processing (4)
  • docs/src/content/docs/dev/features/image-previews.mdx
  • docs/src/content/docs/features/image-previews.mdx
  • src/rovr/components/iterm2_image.py
  • src/rovr/config/schema.json
📝 Walkthrough

Walkthrough

Changes

iTerm2 image support

Layer / File(s) Summary
Renderer contract and inline image rendering
src/rovr/classes/config.pyi, src/rovr/components/iterm2_image.py, tests/test_iterm2_image.py
Adds the ITerm2 protocol type, terminal detection, OSC-1337 rendering, image caching and transformations, with tests for encoding and capability checks.
Protocol selection and first-launch wiring
src/rovr/core/preview_container.py, src/rovr/first_launch.py
Selects the ITerm2 renderer explicitly or automatically for supported terminals, and adds the protocol to first-launch configuration.
Configuration schema and protocol documentation
src/rovr/config/schema.json, docs/src/content/docs/dev/features/image-previews.mdx, docs/src/content/docs/features/image-previews.mdx
Updates valid protocol values, configuration examples, and documentation for automatic ITerm2 selection.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding iTerm2 inline image support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • The ITerm2Image.image setter is implemented with super(__class__, type(self)).image.fset, which is quite non-standard and hard to follow; consider calling the base setter more explicitly (e.g. BaseImage.image.fset(self, value)) or restructuring so a simple super().image = value works.
  • The auto-detection logic for iTerm2 (is_supported) now lives in both preview_container and first_launch with slightly different triggers (not image_protocol vs "auto"); consider centralizing this decision in one helper to avoid future drift in behavior between the two code paths.
  • The mapping between configuration values and protocol keys uses different casings (e.g. config "ITerm2" vs "iterm2" key in prot_to_timg); it would be more robust to normalize protocol names in a single place (e.g. lowercasing) before using them as keys.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `ITerm2Image.image` setter is implemented with `super(__class__, type(self)).image.fset`, which is quite non-standard and hard to follow; consider calling the base setter more explicitly (e.g. `BaseImage.image.fset(self, value)`) or restructuring so a simple `super().image = value` works.
- The auto-detection logic for iTerm2 (`is_supported`) now lives in both `preview_container` and `first_launch` with slightly different triggers (`not image_protocol` vs "auto"); consider centralizing this decision in one helper to avoid future drift in behavior between the two code paths.
- The mapping between configuration values and protocol keys uses different casings (e.g. config `"ITerm2"` vs `"iterm2"` key in `prot_to_timg`); it would be more robust to normalize protocol names in a single place (e.g. lowercasing) before using them as keys.

## Individual Comments

### Comment 1
<location path="src/rovr/components/iterm2_image.py" line_range="97-100" />
<code_context>
+class ITerm2Image(BaseImage, Renderable=_NoopRenderable):
+    """Render an image through the iTerm2 Inline Image Protocol."""
+
+    @override
+    @BaseImage.image.setter
+    def image(self, value: StrOrBytesPath | IO[bytes] | PILImage.Image | None) -> None:
+        super(__class__, type(self)).image.fset(self, value)
+        self.refresh(recompose=True)
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `image` setter uses an unusual `super(__class__, type(self))` pattern that likely doesn't call the intended base property setter

Using `super(__class__, type(self)).image.fset(self, value)` means `super` is bound to the class, not the instance, and effectively resolves as `super(ITerm2Image, ITerm2Image)`. This is non-idiomatic and may bypass or misinvoke `BaseImage.image`. Prefer calling the base setter explicitly, e.g. `BaseImage.image.fset(self, value)`, then `self.refresh(recompose=True)` to keep the behavior clear and correct.
</issue_to_address>

### Comment 2
<location path="src/rovr/components/iterm2_image.py" line_range="204" />
<code_context>
+        ]
+
+    def _get_clear_style(self) -> Style:
+        _, color = self.background_colors
+        return Style(bgcolor=color.rich_color)
</code_context>
<issue_to_address>
**issue (bug_risk):** `_get_clear_style` assumes a background color is always defined, which may not be true

If no background is set on the widget or its ancestors, `self.background_colors` may contain `None`, so `color.rich_color` would raise an `AttributeError`. Please handle the `color is None` case (e.g., default `Style()` or app background) to avoid runtime errors when clearing the region.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +97 to +100
@override
@BaseImage.image.setter
def image(self, value: StrOrBytesPath | IO[bytes] | PILImage.Image | None) -> None:
super(__class__, type(self)).image.fset(self, value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): The image setter uses an unusual super(__class__, type(self)) pattern that likely doesn't call the intended base property setter

Using super(__class__, type(self)).image.fset(self, value) means super is bound to the class, not the instance, and effectively resolves as super(ITerm2Image, ITerm2Image). This is non-idiomatic and may bypass or misinvoke BaseImage.image. Prefer calling the base setter explicitly, e.g. BaseImage.image.fset(self, value), then self.refresh(recompose=True) to keep the behavior clear and correct.

Comment thread src/rovr/components/iterm2_image.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
docs/src/content/docs/features/image-previews.mdx (1)

41-46: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use the schema key protocol, not image_protocol.

The example currently writes [interface.image_viewer].image_protocol, but the schema only permits protocol in this table. Users copying it will receive an invalid configuration; update both the snippet and the following prose.

Suggested correction
-  code={`[interface.image_viewer]\nimage_protocol = "Sixel"`}
+  code={`[interface.image_viewer]\nprotocol = "Sixel"`}
...
-available `image_protocol` values are:
+available `protocol` values are:
🤖 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 `@docs/src/content/docs/features/image-previews.mdx` around lines 41 - 46,
Update the image viewer configuration example and its accompanying prose in the
image previews documentation to use the schema key protocol instead of
image_protocol. Keep the existing [interface.image_viewer] table and supported
protocol values unchanged.
🧹 Nitpick comments (1)
src/rovr/components/iterm2_image.py (1)

126-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use contextlib.suppress for the expected NoScreen path.

Could we factor the active-screen rendering branch so this expected lifecycle condition uses contextlib.suppress(NoScreen) while retaining the [] fallback? As per coding guidelines, “Use contextlib.suppress for expected errors”.

🤖 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/rovr/components/iterm2_image.py` around lines 126 - 130, Update the
active-screen rendering branch in the relevant image-rendering method to use
contextlib.suppress(NoScreen) around the self.screen.is_active access, while
preserving the [] fallback for inactive, missing, or unavailable screens. Remove
the explicit try/except NoScreen and add the required contextlib import.

Source: Coding guidelines

🤖 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 `@docs/src/content/docs/dev/features/image-previews.mdx`:
- Line 32: Update the image-preview capability table to add an iTerm2 column,
and list the supported terminals using the exact “iTerm2” and “WezTerm”
capitalization so it matches the Auto-selection behavior described below.

In `@src/rovr/components/iterm2_image.py`:
- Around line 34-38: The iTerm2 runtime detection in iterm2_image.py should
recognize the advertised TERM_PROGRAM value by replacing the iTerm2 entry with
iTerm.app in the guard around the visible stdout check. Update the corresponding
assertions in tests/test_iterm2_image.py lines 22-29 to use iTerm.app as well;
no other renderer behavior should change.

---

Outside diff comments:
In `@docs/src/content/docs/features/image-previews.mdx`:
- Around line 41-46: Update the image viewer configuration example and its
accompanying prose in the image previews documentation to use the schema key
protocol instead of image_protocol. Keep the existing [interface.image_viewer]
table and supported protocol values unchanged.

---

Nitpick comments:
In `@src/rovr/components/iterm2_image.py`:
- Around line 126-130: Update the active-screen rendering branch in the relevant
image-rendering method to use contextlib.suppress(NoScreen) around the
self.screen.is_active access, while preserving the [] fallback for inactive,
missing, or unavailable screens. Remove the explicit try/except NoScreen and add
the required contextlib import.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e7a46750-8d67-40b6-bbbb-b4f37e5c1462

📥 Commits

Reviewing files that changed from the base of the PR and between b9f73f4 and 40bfe53.

📒 Files selected for processing (8)
  • docs/src/content/docs/dev/features/image-previews.mdx
  • docs/src/content/docs/features/image-previews.mdx
  • src/rovr/classes/config.pyi
  • src/rovr/components/iterm2_image.py
  • src/rovr/config/schema.json
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • tests/test_iterm2_image.py
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: Test on (windows-x64, Python 3.13)
  • GitHub Check: Test on (windows-arm, Python 3.14)
  • GitHub Check: Test on (windows-arm, Python 3.13)
  • GitHub Check: format_and_alert
  • GitHub Check: Sourcery review
⚠️ CI failures not shown inline (1)

GitHub Actions: Post Commit Checks and Pushes / 0_format_and_push_changes.txt: feat: add iTerm2 inline image support

Conclusion: failure

View job details

##[group]Run if ! git diff --quiet; then
 �[36;1mif ! git diff --quiet; then�[0m
 �[36;1m  echo "::error::Formatting changes detected. Please run 'ruff format .' and commit the changes."�[0m
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Python 3.12 or newer is required.
Use double quotes, four-space indentation, and Ruff as the formatter; there is no strict line-length limit.
Use absolute imports only; do not use relative imports.
All functions must have type annotations; use ty for type checking rather than mypy.
Do not use if TYPE_CHECKING blocks.
Use CamelCase for classes, snake_case for functions and variables, UPPER_CASE for constants, and a leading underscore for private names.
Use specific exceptions instead of bare except:; define custom exceptions in rovr/classes/exceptions.py. Use contextlib.suppress for expected errors.

Files:

  • tests/test_iterm2_image.py
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Tests use pytest; test files follow the test_*.py naming convention and shared fixtures belong in conftest.py.

Files:

  • tests/test_iterm2_image.py
src/rovr/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/rovr/**/*.py: For custom Textual keybindings, inherit from widgets with inherit_bindings=False.
Use @on(EventType) decorators or on_event methods for Textual event handling.
Use @work for asynchronous background work, adding thread=True for blocking tasks.
When modifying widgets from a thread, use call_from_thread, call_next, call_later, or call_after_refresh; reactive variables and properties are exempt.
Do not define functions in the same file as multiprocessing.ProcessPoolExecutor or multiprocessing.Process; place worker functions in a separate module to avoid configuration reload and stdout corruption.
Textual event handlers do not need to call super(); call prevent_default() when an event should not bubble.
Using subprocess.run(shell=True) is allowed because command execution is controlled by the user through configuration.

Files:

  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
🧠 Learnings (8)
📚 Learning: 2025-12-11T03:10:54.344Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 167
File: src/rovr/footer/process_container.py:346-346
Timestamp: 2025-12-11T03:10:54.344Z
Learning: In Python/Textual code, when calling checker_wrapper() or any other method that starts workers from within a work(thread=True) decorated method, do not wrap the call in call_from_thread. Starting workers is thread-safe in Textual and the work decorator handles threading. Only UI updates (e.g., DOM modifications) from worker threads require call_from_thread. Apply this guideline to Python files across the codebase where worker-start calls occur; do not wrap such calls with call_from_thread.

Applied to files:

  • tests/test_iterm2_image.py
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2026-01-24T02:10:57.940Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 203
File: src/rovr/first_launch.py:273-295
Timestamp: 2026-01-24T02:10:57.940Z
Learning: In the rovr codebase (Python/Textual), App.notify() is thread-safe and may be called directly from worker threads decorated with work(thread=True) without wrapping in call_from_thread(). Only DOM modifications require call_from_thread from worker threads. Apply this guideline to Python files across the codebase (not just a single file) and avoid unnecessary call_from_thread() wrappers when invoking App.notify() from worker threads.

Applied to files:

  • tests/test_iterm2_image.py
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2026-01-30T13:54:15.110Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 207
File: src/rovr/first_launch.py:96-107
Timestamp: 2026-01-30T13:54:15.110Z
Learning: In Python projects using the Textual framework, async workers decorated with work are automatically cancelled when the parent screen/widget is dismissed or unmounted. Do not add manual cancellation in cleanup/dismiss handlers for these workers; rely on Textual to cancel them. Verify this behavior against the project’s Textual version before relying on it.

Applied to files:

  • tests/test_iterm2_image.py
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2026-02-27T09:15:49.243Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 229
File: src/rovr/functions/system_clipboard.py:143-143
Timestamp: 2026-02-27T09:15:49.243Z
Learning: In Python files that display error messages in toast/notification UI (e.g., ClipboardError and similar exceptions), escape unescaped square brackets in the message so the toast system does not interpret them as markup. Specifically render the content with double backslashes before brackets, e.g. "\\[{type(exc).__name__}]" in the toast description. This guideline applies to all Python files that format and show exception details in toasts across the codebase.

Applied to files:

  • tests/test_iterm2_image.py
  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2026-03-01T10:35:17.682Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 233
File: tests/test_functions_path.py:21-21
Timestamp: 2026-03-01T10:35:17.682Z
Learning: In Python test files (using the Textual framework), use an assertion like Static(id=val, classes=val) to verify that the value contains only characters valid for Textual IDs and classes. The test should confirm initialization succeeds without raising BadIdentifier (do not test attribute storage). This pattern is applicable to tests that validate Textual-compatible identifier strings returned by utilities (e.g., compress/decompress). Apply this guideline to test files under the tests/ directory (e.g., tests/**/*.py) where relevant.

Applied to files:

  • tests/test_iterm2_image.py
📚 Learning: 2025-12-11T03:11:24.387Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 167
File: src/rovr/screens/fd_search.py:205-209
Timestamp: 2025-12-11T03:11:24.387Z
Learning: In src/rovr/screens/fd_search.py, FileSearch.FILTER_TYPES should be implemented as a class attribute (not an instance attribute) to persist filter state across all modal instances within the same session. This enables remember/restore of filter settings when the modal is dismissed and reopened. Ensure this behavior is intentional, document the session lifecycle implications, and include tests verifying persistence across modal reopenings, as well as proper reset or isolation when a new session starts. If applicable, consider thread-safety and potential memory growth due to long-lived state.

Applied to files:

  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2025-12-11T03:11:27.833Z
Learnt from: NSPC911
Repo: NSPC911/rovr PR: 167
File: src/rovr/screens/fd_search.py:205-209
Timestamp: 2025-12-11T03:11:27.833Z
Learning: In src/rovr/screens/fd_search.py, document that the FileSearch class uses a class-level FILTER_TYPES dict to persist filter settings across multiple modal invocations within a single application session. Ensure this pattern is intentional by clarifying the lifecycle of the shared state (lifetime of the process/session) and the implications of mutable shared state (thread-safety, potential memory retention). Consider alternatives (per-instance state, dedicated session storage, explicit reset method) and add tests to verify persistence, correctness after modal reopenings, and safe reset behavior. Also mention any framework or runtime constraints that affect shared-class state.

Applied to files:

  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
📚 Learning: 2026-01-03T14:17:18.132Z
Learnt from: lazysegtree
Repo: NSPC911/rovr PR: 184
File: src/rovr/core/preview_container.py:280-283
Timestamp: 2026-01-03T14:17:18.132Z
Learning: In src/rovr/core/preview_container.py, mypy may not recognize that pdfinfo_from_path's poppler_path parameter accepts None even though the library signature is Optional[Union[str, pathlib.PurePath]]. If you encounter a type error, explicitly cast the argument to Optional[Union[str, pathlib.PurePath]] before passing it (e.g., from typing import cast; pdfinfo_from_path(path, poppler_path=cast(Optional[Union[str, pathlib.PurePath]], None)) or cast the argument to the expected type). This documents the typing limitation and provides a repeatable workaround for similar cases.

Applied to files:

  • src/rovr/core/preview_container.py
  • src/rovr/first_launch.py
  • src/rovr/components/iterm2_image.py
🔇 Additional comments (5)
src/rovr/core/preview_container.py (1)

33-34: LGTM!

Also applies to: 50-57

src/rovr/first_launch.py (1)

40-41: LGTM!

Also applies to: 51-54, 61-64, 274-274

src/rovr/config/schema.json (1)

296-296: LGTM!

docs/src/content/docs/dev/features/image-previews.mdx (1)

41-46: LGTM!

src/rovr/classes/config.pyi (1)

826-833: LGTM!

Also applies to: 843-844

Comment thread docs/src/content/docs/dev/features/image-previews.mdx
Comment on lines +34 to +38
return bool(
sys.__stdout__
and sys.__stdout__.isatty()
and os.environ.get("TERM_PROGRAM") in ("iTerm2", "WezTerm")
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locate files:"
git ls-files | rg '(^|/)(iterm2_image\.py|test_iterm2_image\.py)$|src/rovr/components/iterm2_image.py|tests/test_iterm2_image.py' || true

echo
echo "Production implementation outline:"
if [ -f src/rovr/components/iterm2_image.py ]; then
  wc -l src/rovr/components/iterm2_image.py
  ast-grep outline src/rovr/components/iterm2_image.py || true
  echo "--- production ---"
  cat -n src/rovr/components/iterm2_image.py
fi

echo
echo "Test implementation outline:"
if [ -f tests/test_iterm2_image.py ]; then
  wc -l tests/test_iterm2_image.py
  ast-grep outline tests/test_iterm2_image.py || true
  echo "--- tests ---"
  cat -n tests/test_iterm2_image.py
fi

echo
echo "Usages / references:"
rg -n 'is_supported\(|ITerm2Image|Term_PROGRAM|iterm2|WezTerm|TERM_PROGRAM' src tests 2>/dev/null || true

Repository: NSPC911/rovr

Length of output: 12276


🌐 Web query:

iTerm2 TERM_PROGRAM iTerm.app man iTerm

💡 Result:

The environment variable TERM_PROGRAM is set by iTerm2 to the value iTerm.app [1][2][3]. This variable is used to identify the terminal emulator currently in use, allowing shell configuration files (such as.bashrc or.zshrc) and scripts to detect the environment and conditionally apply terminal-specific settings, themes, or behaviors [2][4][5]. While the query mentions man iTerm, please note that there is no standard manual page for the terminal emulator itself named iTerm. The official documentation for iTerm2 is available online at the iTerm2 website rather than through the local system man command [6][7]. Summary of usage: 1. Identification: You can verify this in your terminal by running echo $TERM_PROGRAM [3][4]. 2. Scripting: It is commonly used in conditional logic within shell profiles, for example: if [ "$TERM_PROGRAM" = "iTerm.app" ]; then # Apply iTerm2-specific configurations fi [3][5] 3. Limitations: Because environment variables are not always passed to remote hosts (e.g., via SSH), relying on TERM_PROGRAM for remote sessions may not be reliable [1][4]. For further assistance, the official iTerm2 documentation is the primary resource for features, scripting, and advanced usage [6][7].

Citations:


Detect iTerm2 using its advertised TERM_PROGRAM value.

TERM_PROGRAM is advertised as iTerm.app, so the current check can return False in iTerm2 and leave image selection on another renderer. Use iTerm.app in both the runtime guard and the supporting test assertions.

Proposed fix
-        and os.environ.get("TERM_PROGRAM") in ("iTerm2", "WezTerm")
+        and os.environ.get("TERM_PROGRAM") in ("iTerm.app", "WezTerm")
-@pytest.mark.parametrize("terminal", ["iTerm2", "WezTerm"])
+@pytest.mark.parametrize("terminal", ["iTerm.app", "WezTerm"])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return bool(
sys.__stdout__
and sys.__stdout__.isatty()
and os.environ.get("TERM_PROGRAM") in ("iTerm2", "WezTerm")
)
return bool(
sys.__stdout__
and sys.__stdout__.isatty()
and os.environ.get("TERM_PROGRAM") in ("iTerm.app", "WezTerm")
)
📍 Affects 2 files
  • src/rovr/components/iterm2_image.py#L34-L38 (this comment)
  • tests/test_iterm2_image.py#L22-L29
🤖 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/rovr/components/iterm2_image.py` around lines 34 - 38, The iTerm2 runtime
detection in iterm2_image.py should recognize the advertised TERM_PROGRAM value
by replacing the iTerm2 entry with iTerm.app in the guard around the visible
stdout check. Update the corresponding assertions in tests/test_iterm2_image.py
lines 22-29 to use iTerm.app as well; no other renderer behavior should change.

@NSPC911
NSPC911 merged commit 2f88b24 into master Jul 29, 2026
31 of 32 checks passed
@NSPC911
NSPC911 deleted the it2iip branch July 29, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants