Closed
Conversation
There was a problem hiding this comment.
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-placeholdertogreger--tool-result-placeholderand adds agreger-tool-result-generatingtext property - Adds logic in
greger.elandgreger-ui.elto 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 togreger--tool-result-placeholderso 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) | ||
|
|
There was a problem hiding this comment.
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)) |
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
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.
No description provided.