Skip to content

gptel-transient: Use proper ellipsis for consistent UI #640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

pabl0
Copy link
Contributor

@pabl0 pabl0 commented Feb 13, 2025

  • gptel-transient.el (gptel-option-overlaid): Use a proper ellipsis (…) instead of three ASCII dots (...) when truncating the user instruction, provided Unicode is displayable and `truncate-string-ellipsis' is nil. This change allows for two additional characters and enhances visual appeal and consistency.

@@ -532,7 +532,7 @@ Also format its value in the Transient menu."
(add-hook 'transient-exit-hook ov-clear-hook)))
;; Updating transient menu display
(if value
(propertize (concat argument (truncate-string-to-width value 25 nil nil "..."))
(propertize (concat argument (truncate-string-to-width value 39 nil nil 'ellipsis))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you changed the width to 39?

Also please just use t instead of 'ellipsis. I know gptel uses both t and 'ellipsis across its code for this (mostly t). I plan to switch to just t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you changed the width to 39?

It was explained in commit msg of second version. Basically it uses all available space without making the default transient more wide:

ellipsis39

But of course the width depends on other settings as well so 39 feels way too specific. Changed it to 35, or do you think it should be as short as possible? Of course the full directive is visible in the actual buffer as well.

ellipsis35

Also please just use t instead of 'ellipsis. I know gptel uses both t and 'ellipsis across its code for this (mostly t). I plan to switch to just t.

Yes I agree it's better, done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is of course transient-force-single-column issue as you mentioned in #630. However the other parts in the transient also need some tweaking to fit into a very narrow side window:

single-column

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

35 is fine, I'll merge this. But I wonder if it's possible to show the additional instruction on the following line, where it can be wider. I remember trying and failing to do that.

* gptel-transient.el (gptel-option-overlaid): Use a proper
ellipsis (…) instead of three ASCII dots (...) when truncating
the user instruction, provided Unicode is displayable and
`truncate-string-ellipsis' is nil.  This change allows for two
additional characters and enhances visual appeal and consistency.
Also, increase width to 35 to use most available space without
making the whole tansient wider.  Fix a typo in docstring.
@karthink karthink merged commit 970debe into karthink:master Feb 14, 2025
marcbowes pushed a commit to marcbowes/gptel that referenced this pull request Mar 19, 2025
* gptel-transient.el (gptel-option-overlaid): Use a proper
ellipsis (…) instead of three ASCII dots (...) when truncating
the user instruction, provided Unicode is displayable and
`truncate-string-ellipsis' is nil.  This change allows for two
additional characters and enhances visual appeal and consistency.
Also, increase width to 35 to use most available space without
making the whole tansient wider.  Fix a typo in docstring.
@pabl0 pabl0 deleted the ellipsis branch March 22, 2025 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants