Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to file from context buffer not working. #482

Closed
1 task done
metachip opened this issue Nov 24, 2024 · 11 comments
Closed
1 task done

Jump to file from context buffer not working. #482

metachip opened this issue Nov 24, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@metachip
Copy link

Please update gptel first -- errors are often fixed by the time they're reported.

  • I have updated gptel to the latest commit and tested that the issue still exists

Bug Description

Jump to file from context buffer not working. This appears to work for buffers and regions, but not files.

Backend

None

Steps to Reproduce

When I add a file to context, and then try to jump to the file from the context window, an error occurs.

The reported error is

funcall-interactively: Wrong type argument: overlayp, "~/Downloads/Roadmap-V1.png"

Additional Context

Emacs 28.2

Backtrace

Debugger entered--Lisp error: (wrong-type-argument overlayp nil)
  gptel-context-visit()
  funcall-interactively(gptel-context-visit)
  command-execute(gptel-context-visit)

Log Information

No response

@metachip metachip added the bug Something isn't working label Nov 24, 2024
@metachip
Copy link
Author

Another minor problem likely to be in the same part of the source code: The gptel-context buffer is not updated when context is deleted. That is, when context is deleted using =d= and =C-c C-c= the context buffer is not updated. The deleted context is still shown. The context buffer only appears to be updated when =gptel-add= or =gptel-add-file= is used. Doing a manual update =g= in the context also updates the context buffer.

@karthink
Copy link
Owner

funcall-interactively: Wrong type argument: overlayp, "~/Downloads/Roadmap-V1.png"

Thanks, I'll fix this.

That is, when context is deleted using =d= and =C-c C-c= the context buffer is not updated

What do you mean by the "context buffer" here? The buffer showing all the context chunks, or the source buffer for one of these chunks?

@metachip
Copy link
Author

The buffer showing all the context chunks.

@karthink
Copy link
Owner

Then I don't understand this:

That is, when context is deleted using =d= and =C-c C-c= the context buffer is not updated

After C-c C-c the context buffer goes away. So what do you mean by "it's not updated"?

@metachip
Copy link
Author

Not if it's open in two windows or one (re)selects it without adding anything else to it. It still exists, with stale data. Not a major problem, but confusing.

@karthink
Copy link
Owner

Got it, thanks for reporting this too. I'll fix both bugs when I can.

karthink added a commit that referenced this issue Nov 24, 2024
* gptel-context.el (gptel-context-confirm, gptel-context-visit):
- Refresh the context buffer when confirming deletions.
- Jump to files correctly when calling `gptel-context-visit'.
@karthink
Copy link
Owner

I've pushed a fix to both bugs. Please test.

@metachip
Copy link
Author

Both bugs appear to be fixed. Thank you!

There appears to remain some stateful problems with the context buffer though.

Reproduce: open the content buffer in a separate window. Add context to it and that buffer will not update. This looks very similar to the problem you just fixed, where the buffer was not updated when items were deleted from it. It's also not updated when context is added to it.

However, when you reopen/revisit context via the gptel menu C command context is correctly updated. That is, updates occur on view events not when the context is changed.

Given the modus operandi for the system at the moment, where the presumption seems to be that the context buffer is only shown upon gptel view events, and hidden again automatically based on actions taken via the gptel events, one normally wouldn't see this problem.

However, if context buffers become things one manages in multiplicity and explicitly in future (as I would suggest), their update should be synchronized to events that actually add or remove context, and not upon events related to viewing of context buffers.

That is, the context buffer should always reflect the correct context state given the most recent modifications made to the context, regardless of whether the context buffer(s) is(are) visible or not.

@karthink
Copy link
Owner

karthink commented Nov 25, 2024

Reproduce: open the content buffer in a separate window. Add context to it and that buffer will not update. This looks very similar to the problem you just fixed, where the buffer was not updated when items were deleted from it. It's also not updated when context is added to it.

This is how other listings like buffer-menu, ibuffer and dired work -- you have to refresh the display with g. This can be addressed but I think this can wait until the design is worked on more -- the fix to automate this won't work well with having multiple context lists (as you suggest in #475.)

@metachip
Copy link
Author

This is how other listings like buffer-menu, ibuffer and dired work -- you have to refresh the display with g.

Fair enough, hitting g is easy enough.

@metachip
Copy link
Author

I consider this bug (jump from context) close with thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants