Skip to content

Conversation

@mkh-user
Copy link
Member

@mkh-user mkh-user commented Sep 15, 2025

Type of Change

  • New feature
  • Bug fix
  • UI/UX improvement

Description

Keeps vertical and horizontal scroll value in Global.set_editor_text() when keep_carets is true for better UX, because when your selection doesn't change you need to keep scroll value.

Testing

Keeps scroll value as should be.

Impact

Nothing

Additional Information

Nothing

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings
  • Changes were added to CHANGELOG

Summary by CodeRabbit

  • Bug Fixes

    • Fixed editor view jumping when text is updated programmatically; the editor now preserves scroll position when updates also preserve cursor/caret placement.
  • Documentation

    • Updated the Unreleased section of the changelog with a “Fixed” entry describing the editor scroll correction.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Captures and restores editor horizontal/vertical scroll when calling set_editor_text(text, keep_carets) with keep_carets == true. Adds a new "Fixed" subsection to CHANGELOG.md documenting the change.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Added a new "Fixed" subsection under the [unreleased] heading documenting: "Preserve editor scroll when text is updated programmatically when keep_carets is true (PR #101)". Documentation-only.
Editor scroll preservation
core/autoload/global.gd
Updated set_editor_text(text, keep_carets) to read current horizontal/vertical scrollbar values into a local Vector2 when keep_carets is true, perform the text update and caret restoration, then re-apply saved scroll values. No public API/signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant Global as Global.set_editor_text
  participant Editor as Editor/Text
  participant Carets as Caret Manager
  participant Scroll as Scrollbars

  Caller->>Global: set_editor_text(text, keep_carets)
  alt keep_carets == true
    Global->>Scroll: read scroll.x / scroll.y
    Note over Global,Scroll: capture scroll (Vector2)
    Global->>Editor: set text
    Global->>Carets: restore carets
    Global->>Scroll: apply saved scroll.x / scroll.y
    Note over Global,Scroll: scroll restored after caret restore
  else keep_carets == false
    Global->>Editor: set text
  end
  Global-->>Caller: return
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I twitch my whiskers, save the view,
A tiny hop, the scroll stays true.
Carets fixed, the text refreshed,
My burrow neat, no chaos left.
Nibble done—PR tucked, adieu! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Auto scroll restore" is concise and directly reflects the primary change described in the PR (restoring editor scroll when set_editor_text preserves carets), so it communicates the main intent to reviewers and in project history. It is short, focused, and related to both the code change and the documented changelog entry.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18dedcd and 3732a35.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • core/autoload/global.gd (1 hunks)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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[bot]

This comment was marked as resolved.

@mkh-user mkh-user self-assigned this Sep 15, 2025
@mkh-user mkh-user added this to the Text Forge 0.2 milestone Sep 15, 2025
@mkh-user mkh-user added the bug A confirmed issue causing incorrect behavior or unexpected results label Sep 15, 2025
@mkh-user mkh-user moved this to In Progress in Release: Text Forge 1.0 Sep 15, 2025
@mkh-user mkh-user merged commit 50acec6 into Main Sep 16, 2025
1 of 2 checks passed
@mkh-user mkh-user deleted the auto-scroll-restore branch September 16, 2025 13:51
@github-project-automation github-project-automation bot moved this from In Progress to Completed in Release: Text Forge 1.0 Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug A confirmed issue causing incorrect behavior or unexpected results

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

2 participants