chatgpt-shell icon indicating copy to clipboard operation
chatgpt-shell copied to clipboard

Make display-buffer-alist configuration customizable

Open DivineDominion opened this issue 1 year ago • 6 comments

Currently, the built-in display-buffer-alist settings are applied on the fly and thus put in front of the list -- so they always win:

https://github.com/xenodium/chatgpt-shell/blob/39dd8e7415ebe6d836a1d721337019cfea89f5ad/chatgpt-shell.el#L1182-L1188

This makes customization of e.g. the direction in which to display the buffer impossible.

WDYT about making the value a defcustom with a default of

             (cons (regexp-quote (buffer-name buffer))
                   '((display-buffer-reuse-window display-buffer-in-direction)
                     (dedicated . t)
                     (reusable-frames . visible)
                     (direction . left)
                     (window-width . 0.35)))

so users can use customize-variable for tweaks, and also nil to disable auto-setting this?

DivineDominion avatar Jan 13 '24 10:01 DivineDominion

WDYT about making the value a defcustom with a default of

We may have to make the defcustom accept a function, since it'd have to resolve "(regexp-quote (buffer-name buffer))" to the current buffer name. The challenge being that these buffers can change in name, specially if either chatgpt-shell-swap-model-version or chatgpt-shell-swap-system-prompt are invoked on the shell.

settings are applied on the fly and thus put in front of the list

What if we uniquely append instead? Would the user customization win?

xenodium avatar Jan 14 '24 11:01 xenodium

WDYT about making the value a defcustom with a default of

We may have to make the defcustom accept a function, since it'd have to resolve "(regexp-quote (buffer-name buffer))" to the current buffer name. The challenge being that these buffers can change in name, specially if either chatgpt-shell-swap-model-version or chatgpt-shell-swap-system-prompt are invoked on the shell.

Makes sense!

For what it's worth, I believe a regexp based matcher for ^*chatgpt* prefix and compose$ suffix would be enough for the built-in setting (it applies the same settings for all model versions anyway). And if users want to customize where 3.5-turbo is displayed, adding an alist item is simple enough (plus maybe disabling the default one)

What if we uniquely append instead? Would the user customization win?

To be honest, I believe that relying on the list position is a quick fix that works in the user configuration, but should not be the default behavior of a library (if we can help it) because it's so brittle! User customizations can change the list in many ways and then we have the same problem but for different reasons

DivineDominion avatar Jan 15 '24 08:01 DivineDominion

To be honest, I believe that relying on the list position is a quick fix that works in the user configuration

Sounds good. Let's go for something more explicit...

For what it's worth, I believe a regexp based matcher for ^chatgpt prefix and compose$ suffix would be enough for the built-in setting (it applies the same settings for all model versions anyway). And if users want to customize where 3.5->turbo is displayed, adding an alist item is simple enough (plus maybe disabling the default one)

Eventually, I'd like to make it much easier to support other llm backends by virtue of moving logic to shell-maker. If at all possible, I'd trying to stay away from adding chatgpt-specific logic if possible.

Lemme see if I can add the function-based option to shell-maker.

xenodium avatar Jan 18 '24 20:01 xenodium

This sounds like a sensible decision! Ping me if I can help in any way.

DivineDominion avatar Jan 27 '24 15:01 DivineDominion

Sorry for the delay.

It'll be quite some time until I rework the compose feature and move it to shell-maker, so better to remove the automatic entry in display-buffer-alist for the time being (e2073d9).

I think this should unblock ya?

xenodium avatar Jan 29 '24 09:01 xenodium

Thanks! Don't worry, I'm fine in my own setup

DivineDominion avatar Jan 29 '24 12:01 DivineDominion

While I haven't moved the compose functionality to shell-maker, the compose buffers do use their own major mode now, which simplifies display-buffer-alist usage. For example:

(add-to-list 'display-buffer-alist
             (cons '(major-mode . chatgpt-shell-prompt-compose-mode)
                   '((display-buffer-reuse-window
                      display-buffer-in-direction
                      display-buffer-select)
                     (reusable-frames . visible)
                     (direction . left)
                     (window-width . 0.35))))

I think this should hopefully address the original request to Make display-buffer-alist configuration customizable. Gonna close this issue, but please reopen if ya reckon there's something missing.

xenodium avatar Jun 23 '24 11:06 xenodium

Thank you! I'll play around with this.

DivineDominion avatar Jun 24 '24 08:06 DivineDominion