gptel-transient: Use width of the full frame instead of window

[pabl0]

• gptel-transinent.el: (gptel--read-with-prefix, gptel-system-prompt--format, gptel--setup-directive-menu, gptel-tools): Use frame-width' instead of window-width' when truncating things when using the minibuffer. Otherwise they get truncated too early when the user has horizontally split windows.

[karthink]

See #317 and #301

[pabl0]

See #317 and #301

A-ha, apologies for not checking closed PRs for such an obvious improvement idea.

Would it be too hacky approach if frame-width would be used in case the following is t?

;; the current default value not likely changing often?
(equal transient-display-buffer-action
    '(display-buffer-in-side-wi	ndow
    (side . bottom)		
    (dedicated . t)		
    (inhibit-same-window . t)))	

Just thinking that most likely folks annoyed by this truncating behavior outnumber the ones who want to customize the variable.

[karthink]

Yup, I think that's too hacky. Technically you want

(equal transient-display-buffer-action
       (apply #'eval (get 'transient-display-buffer-action 'standard-value)))

but this is the kind of "fix" that feels mighty clever in the present, and leaves you scratching your head and cursing yourself in the future. So I want to avoid it.

This can also break in many ways, such as with a display buffer action that still uses the full frame but differs from the default.

The correct solution is to find a way to get the width of the transient window before truncating the system message. I haven't had the time or inclination to investigate this in transient.el or bother tarsius with it, but if you're interested feel free to do so. 🙂

[pabl0]

Yup, I think that's too hacky. Technically you want

(equal transient-display-buffer-action
       (apply #'eval (get 'transient-display-buffer-action 'standard-value)))

but this is the kind of "fix" that feels mighty clever in the present, and leaves you scratching your head and cursing yourself in the future. So I want to avoid it.

Isn't` this just a fancy way of saying

(default-value 'transient-display-buffer-action)

But what if tarsius decides to change it in some subtle way.. so that frame-sizewould no longer be correct? Well I guess it's unlikely that it would suddenly open the transient menus on the right side or something...

This can also break in many ways, such as with a display buffer action that still uses the full frame but differs from the default.

Well, then window-width would be used and you would be back to square one – the situation would be the same as currently? With this hack at least it would work for the most common use case, i.e. user is not an transient.el expert who would tweak such settings.

The correct solution is to find a way to get the width of the transient window before truncating the system message. I haven't had the time or inclination to investigate this in transient.el or bother tarsius with it, but if you're interested feel free to do so. 🙂

Challenge accepted. But oh boy transient.el is complex, so maybe it will be too much for my pay grade.

[karthink]

Isn't this just a fancy way of saying (default-value 'transient-display-buffer-action)

No, these are completely different things. default-value returns the global value of buffer-local variables.

But what if tarsius decides to change it in some subtle way.. so that frame-sizewould no longer be correct? Well I guess it's unlikely that it would suddenly open the transient menus on the right side or something...

See transient-force-single-column. There have been discussions about opening transient in a vertical split when this is turned on.

Challenge accepted. But oh boy transient.el is complex, so maybe it will be too much for my pay grade

👍

I've found understanding it to be straightforward, but remembering how transient works is impossible. If it's been a couple of months since i last looked at it I have to read the whole file again.

[pabl0]

Okay, I believe I finally figured it out. Of course can't promise some weird transient-display-buffer-actionsetup couldn't break things, but I think this is at least more correct than what we have now.

[pabl0]

(Okay not quite there yet but after a good night sleep I'll get there. Turns out the horizontal use case is more tricky + need to check for window-live-p)

[pabl0]

Things I've learned so far trying to get this working:

1. Using window-width is incorrect, window-max-chars-per-line should be used when calculcating layout. In practice, both of them return the same value on graphical Emacs (at least gtk), but in text mode window-width is one too large. See info (elisp) Window Sizes.
2. Simply getting window-width gives the width of the current active buffer where transient was launched. It is correct in the typical case of transient placed on the bottom and no horizontal splits, but breaks if there are C-x 3 splits or if the transient is placed on left/right side.
3. Using (window-max-chars-per-line transient--window)gives the correct number when launching the first level transient menu. However, things get tricky when the user hits s to get into the "set system message" transient. Now something odd happens, and transient--window gets to niland the window gets into some state where it does not exists even in window-list. Of course the first idea is to save the width into a variable, but there is unfortunately an additional challenge: the width of the transient window changes when switching to the next transient. So here I am totally stuck how to get the correct width of the window. Can anyone give me a helping hand?

So, I can get the normal case with transient in the bottom working correctly each time, but it is already in current state broken when the transient is displayed in the right/left (well actually tested just right but assuming the problem is symmetrical).

Perhaps we could just hardcode the width of the system message etc to be the same as the transient menu items when transient menu is on the side? Please note that this is already broken as is.

EDIT: Testing on Emacs 28.2 and 29.4 with transient 0.8.4. Perhaps the transient version in MELPA has improved, but presumably we're not going to rely on bleeding edge MELPA version.