s-kostyaev / ellama

Ellama is a tool for interacting with large language models from Emacs.
GNU General Public License v3.0
359 stars 25 forks source link

Avoid advising global functions #60

Closed Stebalien closed 5 months ago

Stebalien commented 5 months ago

Please try to avoid advising global functions for mode-local functionality. Advice is a last-ditch "there's no other way" method.

s-kostyaev commented 5 months ago

Today I learn. Thank you. I will fix it.

Stebalien commented 5 months ago

Ah, sorry, I was already working on a fix. See #62.

s-kostyaev commented 5 months ago

@Stebalien Merged. But after merge I realized that it breaks request cancel functionality for non-chat and non-instant buffers. Like ellama-add-code, ellama-complete or ellama-make-concise etc.

I will fix it now by enable ellama-instant-mode in this buffers.

s-kostyaev commented 5 months ago

@Stebalien Thank you!

Stebalien commented 5 months ago

I will fix it now by enable ellama-instant-mode in this buffers.

In that case, I'd consider renaming it to something like ellama-request-mode and only turning it on while the request is ongoing (maybe even in ellama-session-mode buffers?).

s-kostyaev commented 5 months ago

Since its changes buffer-local I don't think we should turn it off. But move it into ellama-stream looks promising... Maybe it's good idea

Stebalien commented 5 months ago

Yeah, I guess there's little harm in leaving it on. I was just thinking users might want to bind keys that are only active when actively querying ellama.

s-kostyaev commented 5 months ago

@Stebalien see #67