Skip to content
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

gptel: Silence byte-compilation warnings #657

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pabl0
Copy link
Contributor

@pabl0 pabl0 commented Feb 20, 2025

Silence most low-hanging-fruit byte compilation warnings.

https://gist.github.com/pabl0/b48d221ab50914dc1cbf1196efca82d6

I think these might be real issues needing fixing?

Emacs 29.4:

gptel-rewrite.el:522:10: Warning: ‘gptel--suffix-rewrite’ is for interactive
    use only.
gptel-rewrite.el:529:10: Warning: ‘gptel--suffix-rewrite’ is for interactive
    use only.

Emacs 28.2:

gptel-rewrite.el:365:55: Warning: the function ‘rmc--add-key-description’ is
    not known to be defined.

@karthink
Copy link
Owner

Thanks. I think I fixed the gptel-rewrite warnings.

For the rest I would like to figure out why the byte-compiler thinks the docstrings of the gptel-make-* functions are too wide instead of ignoring them. Any idea?

@pabl0
Copy link
Contributor Author

pabl0 commented Feb 22, 2025

For the rest I would like to figure out why the byte-compiler thinks the docstrings of the gptel-make-* functions are too wide instead of ignoring them. Any idea?

https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-09/msg00713.html

@pabl0
Copy link
Contributor Author

pabl0 commented Feb 22, 2025

Thanks. I think I fixed the gptel-rewrite warnings.

Emacs 28 is lacking the rmc--add-key-description function. Does gptel--rewrite-dispatch work correctly with it just declared but not actually implemented with older emacsen?

gptel/gptel-rewrite.el

Lines 363 to 369 in 5d5610d

(concat
(unless (eq (char-before (overlay-start ov)) ?\n) "\n")
(propertize "REWRITE READY: " 'face 'success)
(mapconcat (lambda (e) (cdr e)) (mapcar #'rmc--add-key-description choices) ", ")
(propertize
" " 'display `(space :align-to (- right ,(1+ (length hint-str)))))
(propertize hint-str 'face 'success)))

@pabl0
Copy link
Contributor Author

pabl0 commented Feb 22, 2025

For the rest I would like to figure out why the byte-compiler thinks the docstrings of the gptel-make-* functions are too wide instead of ignoring them. Any idea?

https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-09/msg00713.html

To clarify:

(car (last (split-string (documentation 'gptel-make-azure) "\n" t)))
==> "(fn NAME &key CURL-ARGS HOST (PROTOCOL \"https\") (HEADER (lambda nil `((\"api-key\" \\, (gptel--get-api-key))))) (KEY 'gptel-api-key) MODELS STREAM ENDPOINT REQUEST-PARAMS)"
(length (car (last (split-string (documentation 'gptel-make-azure) "\n" t))))
==> 168

describe-function wraps it nicely, but I don't think the cl-defun macro can wrap the actual docstring stored.

If I set byte-compile-docstring-max-column to 168, it compiles without warnings. Would that be a more suitable option? I don't think it makes much practical difference.

I believe this issue should go away in future versions of Emacs where the internal docstring presentation of macros will be wrapped, if I understand the emacs-devel thread correctly.

* gptel-antropic (gptel--anthropic-models): Use URL format recognized
by `describe-variable` in docstring.

* gptel-anthropic.el (gptel-make-anthropic): Silence warnings about
docstring (created by a macro) wider than 80 characters with local
variable.  (See Emacs bug#65790.)

* gptel-kaqi.el (gptel--wrap-user-prompt): Ditto.

* gptel-openai.el: (gptel-make-azure): Ditto.

* gptel-openai-extras.el (gptel-make-perplexity): Ditto.

* gptel-ollama.el (gptel--wrap-user-prompt): Wrap a docstring line to
less than 80 characters.

* gptel-org.el (gptel-org--element-lineage-map): Silence warnings
about `org-element-type-p' and `org-element-parent', see karthink#294.
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