Skip to content

handle package-version safely to prevent listp error #344

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
Jan 31, 2025

Conversation

yaaama
Copy link
Contributor

@yaaama yaaama commented Aug 28, 2024

When package-version' is not what we expect (e.g a simple string), a listp` error is thrown and you are unable to view the variable documentation due to it. This now handles it and provides a fallback value.

This was a recurring problem I was facing when opening which-key variables using helpful.
which-key would send a value of "31.0" and then cause this error:

helpful--version-info: Wrong type argument: listp, "1.0"

When `package-version' is not what we expect (e.g a simple string), a `listp`
error is thrown and you are unable to view the variable documentation due to it.
This now handles it and provides a fallback value.
@elken
Copy link

elken commented Jan 5, 2025

I just hit this also and fixed it like this, which I think is preferable as it still shows the package

diff --git a/helpful.el b/helpful.el
index b9ee878..0e73383 100644
--- a/helpful.el
+++ b/helpful.el
@@ -1309,20 +1309,26 @@
   "If SYM has version information, format and return it.
 Return nil otherwise."
   (when (symbolp sym)
-    (let ((package-version
-           (get sym 'custom-package-version))
-          (emacs-version
-           (get sym 'custom-version)))
-      (cond
-       (package-version
-        (format
-         "This variable was added, or its default value changed, in %s version %s."
-         (car package-version)
-         (cdr package-version)))
-       (emacs-version
-        (format
+    (string-join
+     (list
+      (when-let* ((package-version
+		   (get sym 'custom-package-version)))
+	(pcase-let ((`(,package . ,version) (if (listp package-version)
+						package-version
+					      `(,(file-name-base
+						  (symbol-file sym))
+						.
+						,package-version))))
+	  (format
+           "This variable was added, or its default value changed, in %s version %s."
+           package
+           version)))
+      (when-let* ((emacs-version
+		   (get sym 'custom-version)))
+	(format
          "This variable was added, or its default value changed, in Emacs %s."
-         emacs-version))))))
+         emacs-version)))
+     "\n\n")))
 
 (defun helpful--library-path (library-name)
   "Find the absolute path for the source of LIBRARY-NAME.

So feel free to use that instead of "unknown" for a nicer UX if you want

@LemonBreezes
Copy link
Contributor

Please merge some version of this fix someone.

@yaaama
Copy link
Contributor Author

yaaama commented Jan 31, 2025

Please merge some version of this fix someone.

I've merged elkens fix into my master branch

@yaaama yaaama closed this Jan 31, 2025
@yaaama yaaama reopened this Jan 31, 2025
@Wilfred Wilfred merged commit 34328c6 into Wilfred:master Jan 31, 2025
8 checks passed
@Wilfred
Copy link
Owner

Wilfred commented Jan 31, 2025

Thanks :)

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.

4 participants