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. :slightly_smiling_face:

[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

:+1:

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.

[karthink]

Wow, thanks for the comprehensive investigation. I'll test when I can.

gptel currently requires a minimum transient version of 0.7.4, so we don't need to worry about the peculiarities/bugs in older versions.

[pabl0]

Wow, thanks for the comprehensive investigation. I'll test when I can.

My conclusion is that at least with transient.el 0.8.4 getting the correct window width is very hard if not impossible, since the contents is rendered to the " transient" buffer before the window is created.

https://github.com/magit/transient/blob/e5cb1fd7e8d35e264313436f47972acae0819764/lisp/transient.el#L4108-L4111

and then it eventually calls fit-window-to-buffer to do what the function name says.

I'd say the sensible thing is to hard code the width of the system message to be roughly the same as the transient is at the default state. Or to check if transient-display-buffer-action has side . bottom (or top) and then use the frame width, otherwise hard code since the side windows can be almost any size. And if transient-force-single-column is enabled, I guess it needs to be very narrow.

BTW Emacs starts with 80×35 frame by default and the current main transient does not really fit into that nicely. EDIT: so perhaps 80 characters might be a good width to hard code. It is after all the only true VT100 terminal width and compatible with IBM punch cards :)

[karthink]

I'd say the sensible thing is to hard code the width of the system message to be roughly the same as the transient is at the default state

If the default is set to frame-width it breaks like this if displaying transient in a window narrower than the frame:

If the default is set to window-width, it doesn't break if displaying transient in a window as wide as the frame.

Or to check if transient-display-buffer-action has side . bottom (or top) and then use the frame width, otherwise hard code since the side windows can be almost any size. And if transient-force-single-column is enabled, I guess it needs to be very narrow.

This sounds very finicky, I'd prefer not to do this. It's going to break if using transient-force-single-column in a narrow window in any case.

so perhaps 80 characters might be a good width to hard code

Windows can be narrower than that.

Are we back to "there is no good solution"?

[pabl0]

If the default is set to frame-width it breaks like this if displaying transient in a window narrower than the frame:

Okay, what kind of transient-display-buffer-action configuration would that be?

If the default is set to window-width, it doesn't break if displaying transient in a window as wide as the frame.

window-width is still wrong in terminal mode, window-max-per-chars-per-line should be used at lest.

This sounds very finicky, I'd prefer not to do this. It's going to break if using transient-force-single-column in a narrow window in any case.

Of course you would check for that setting and change the behavior accordingly.

Windows can be narrower than that.

Sure, but then the current transient will wrap no matter what. Actually, like I said, it already looks like that with the default frame size of Emacs, doesn't need to be narrower than 80 characters:

Are we back to "there is no good solution"?

Yes I don't think there is a perfect or elegant solution, but I don't think it is a reason not to try to improve the current situation with some compromise. Right not even the most common use case is broken.

[pabl0]

One option is the default main menu be adjusted to two columns instead of three to better fit an 80-character width. However, this change would result in a taller layout that could feel somewhat crowded.

It appears that transient is primarily optimized for use with Magit and generally favors narrow menus, avoiding any lengthy dynamic text fields for the user. I believe it's essential to keep the system message visible in some capacity. Could we explore a different approach where it isn't displayed in the transient itself but instead appears in a separate window on top of the transient?

[karthink]

window-width is still wrong in terminal mode, window-max-per-chars-per-line should be used at lest.

It doesn't make a difference since we don't use window-width, but some number less than that by a generous margin.

Right now even the most common use case is broken

What is the most common use case? I don't think many people are using Emacs at a frame width of 80 chars?

One option is the default main menu be adjusted to two columns instead of three to better fit an 80-character width. However, this change would result in a taller layout that could feel somewhat crowded.

Yes, the current layout is a compromise between width and height chosen after a lot of testing.

It appears that transient is primarily optimized for use with Magit and generally favors narrow menus, avoiding any lengthy dynamic text fields for the user.

Transient menus used by Forge, also by Tarisus, are large:

I believe it's essential to keep the system message visible in some capacity. Could we explore a different approach where it isn't displayed in the transient itself but instead appears in a separate window on top of the transient?

I'm satisfied by the preview of the system message you currently see. If you are using a single column layout or a very narrow window showing a long system message will take up a big chunk of the frame height no matter how you show it.

[pabl0]

Right now even the most common use case is broken

What is the most common use case? I don't think many people are using Emacs at a frame width of 80 chars?

While it’s probably rare for users to utilize 80-column frames on modern wide and large monitors — despite it being the default setting in Emacs — my reference to the "most common use case" primarily points to the fact that many users likely retain the default transient-display-buffer-action setting, which opens Transient at the bottom. Vertical splits are quite frequent, resulting in users often encountering unnecessarily truncated system messages. That said, I’m not sure if anyone genuinely cares, and a single 300-character line is not particularly easy to read anyway.

However, the conclusion still stands that given the current functioning of Transient, there likely isn't an ideal solution to this issue. But we can expect new PRs trying to fix this arrive from time to time from new contributors...