search

karthink / gptel

A simple LLM client for Emacs
GNU General Public License v3.0
1.04k stars 113 forks source link

Avoid clashes with the custom directive key and use an arbitrary unused key in the worst case #219

Closed martenlienen closed 4 months ago

martenlienen commented 4 months ago

At the moment, the transient can assign h to one of your directives and then you need to use the mouse to customize directives on the fly. Also, if all characters of your directive name were already assigned, it would not assign any. With this PR, an arbitrary unused key will be assigned instead.

karthink commented 4 months ago

@martenlienen Thanks for the PR!

Also, if all characters of your directive name were already assigned

Can you give me an example of how this can happen under the current scheme? I'm imagining that a combination (> 2) of similar directive names can lead to this, but I couldn't come up with an example.

martenlienen commented 4 months ago

Consider the case where you have three directives a, b and ab. When you get to ab, both characters have already been taken, so it continues to loop until you get

ELISP> (substring "ab" 2 3)
*** Eval error ***  Args out of range: "ab", 2, 3

so it would actually error, I believe, instead of assigning no key.

karthink commented 4 months ago

Consider the case where you have three directives a, b and ab. When you get to ab, both characters have already been taken, so it continues to loop until you get

ELISP> (substring "ab" 2 3)
*** Eval error ***  Args out of range: "ab", 2, 3

so it would actually error, I believe, instead of assigning no key.

Ah right, thanks for clearing it up.

karthink commented 4 months ago

@martenlienen Thanks for the PR! Merging now.

BTW I think there's one edge case still unaddressed: Consider the directives a, b, ab and c. Per the current logic,

Ideally c would be assigned to c and ab to something else. But I think we can live with this, it's exceedingly unlikely to happen.