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

Fix custom type specifications #239

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tarsius
Copy link
Contributor

@tarsius tarsius commented Sep 20, 2023

On Emacs' "master" branch, the byte-compiler recently learned to recognize more common mistakes in custom type specifications. This pull-requests fixes most of them, but there are two exceptions, you still have to find a solution for:

lib/dirvish/dirvish.el:190:29: Warning: in defcustom for ‘dirvish-open-with-programs’: irregular type ‘(repeat string)’
lib/dirvish/dirvish.el:191:31: Warning: in defcustom for ‘dirvish-open-with-programs’: irregular type ‘(repeat string)’

This pr also includes the commits from my earlier prs, #220 and #227.

@aikrahguzar
Copy link
Contributor

aikrahguzar commented Sep 29, 2023

lib/dirvish/dirvish.el:190:29: Warning: in defcustom for ‘dirvish-open-with-programs’: irregular type ‘(repeat string)’
lib/dirvish/dirvish.el:191:31: Warning: in defcustom for ‘dirvish-open-with-programs’: irregular type ‘(repeat string)’

This pr also includes the commits from my earlier prs, #220 and #227.

I think this warning can be silenced with something like

diff --git a/dirvish.el b/dirvish.el
index 0eb16bd..f3b0b6a 100644
--- a/dirvish.el
+++ b/dirvish.el
@@ -163,8 +163,9 @@ and its ARGS is issued to open the file externally.  The special
 placeholder \"%f\" in the ARGS is replaced by the FILENAME at
 runtime.  Set it to nil disables this feature."
   :group 'dirvish
-  :type '(alist :key-type ((repeat string) :tag "File extensions")
-                :value-type ((repeat string) :tag "External command and args")))
+  :type '(alist :key-type (repeat :tag "File extension" string)
+                :value-type (cons (string :tag "External command")
+                                  (repeat :tag "Args for the command" string))))
 
 (defcustom dirvish-reuse-session t
   "Whether to reuse the hidden sessions.


(defcustom dirvish-nerd-icons-offset 0.00
"Icon's vertical offset used for `nerd-icons' backend.
Set it to nil to use the default offset from `nerd-icons'."
:group 'dirvish :type '(choice (float nil)))
:group 'dirvish :type '(choice (float (const nil))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably be '(choice float (const nil))


(defcustom dirvish-peek-categories '(file project-file library)
"Minibuffer metadata categories to show file preview."
:group 'dirvish :type 'list)
:group 'dirvish :type '(list symbol))
Copy link
Contributor

Choose a reason for hiding this comment

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

Going by the default value the type should probably be (repeat symbol).

@@ -65,7 +65,7 @@ The value can be a symbol or a function that returns a fileset."

(defcustom dirvish-yank-rsync-args '("--archive" "--verbose" "--compress" "--info=progress2")
"The default options for the rsync command."
:type 'list :group 'dirvish)
:type '(list string) :group 'dirvish)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here the type should probably be (repeat string).

@tarsius
Copy link
Contributor Author

tarsius commented Sep 29, 2023

@aikrahguzar Thanks for your feedback. I've adjusted the commit accordingly.

@tarsius
Copy link
Contributor Author

tarsius commented Sep 29, 2023

I've stuck to just :value-type (repeat ... string).

@aikrahguzar
Copy link
Contributor

@tarsius, thanks but I don't see any changes :)

@tarsius
Copy link
Contributor Author

tarsius commented Sep 30, 2023

Ups, forgot to push.

hlissner added a commit to hlissner/dirvish that referenced this pull request Aug 7, 2024
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