Skip to content

Fix tool result highlighting#76

Closed
andreasjansson wants to merge 11 commits intomainfrom
fix-tool-result-highlighting
Closed

Fix tool result highlighting#76
andreasjansson wants to merge 11 commits intomainfrom
fix-tool-result-highlighting

Conversation

@andreasjansson
Copy link
Owner

No description provided.

@andreasjansson andreasjansson requested a review from Copilot July 3, 2025 10:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates tool-result handling so that results still generating are neither folded nor syntax-highlighted, and hides the ID line until generation completes.

  • Renames greger--tool-placeholder to greger--tool-result-placeholder and adds a greger-tool-result-generating text property
  • Adds logic in greger.el and greger-ui.el to skip folding/highlighting while a result is generating, and to clear the property on completion
  • Updates existing tests to use the new placeholder function and adds a UI test for generating behavior

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/greger-ui-test.el Added UI test verifying that generating tool results remain visible and unhighlighted
test/greger-test.el Updated test code to call greger--tool-result-placeholder instead of the old placeholder
greger.el Renamed placeholder function, applied text property, added node lookup/removal logic
greger-ui.el Modified folding and syntax-highlighting functions to respect the generating property
Comments suppressed due to low confidence (1)

greger.el:807

  • The TODO comment still refers to greger--tool-placeholder. Update it to greger--tool-result-placeholder so it matches the renamed function.
    (greger--append-tool-result-text state tool-id tool-result-text t)

Comment on lines +875 to +877
;; Add the generating property to simulate a generating tool result
(put-text-property (point-min) (1+ (point-min)) 'greger-tool-result-generating t)

Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

The test applies the greger-tool-result-generating property at buffer start, but the UI logic checks it at the tool-result node start. Tag the actual placeholder insertion range (or the tool-result node region) to ensure the generating flag is detected.

Suggested change
;; Add the generating property to simulate a generating tool result
(put-text-property (point-min) (1+ (point-min)) 'greger-tool-result-generating t)
;; Add the generating property to the tool-result node region to simulate a generating tool result
(let ((tool-result-start (search-forward "<tool.toolu_999>"))
(tool-result-end (search-forward "</tool.toolu_999>")))
(put-text-property tool-result-start tool-result-end 'greger-tool-result-generating t))

Copilot uses AI. Check for mistakes.
@andreasjansson andreasjansson deleted the fix-tool-result-highlighting branch July 3, 2025 10:56
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