branch: elpa/casual
commit 956a5e9343e17655eefc987853eeed1a44bac342
Author: Philip Kaludercic <[email protected]>
Commit: Charles Choi <[email protected]>
Request to distribute Casual packages on NonGNU ELPA
Stefan Monnier <[email protected]> writes:
>> - Target version 2.14.3 (not yet made) as the version that will include
all
>> changes required by NonGNU ELPA.
>> - Fold Stefan’s patch into 2.14.3.
>
> Sounds good.
>
>> - Fix so that ‘casual.info’ file is auto-generated.
>
> AFAIK the only thing you need here is to `git rm casual.info`.
>
>> - Fix customizable variables that pollute ‘dired' and ‘info’ customizable
>> groups to instead be in the ‘casual’ group.
>
> [ I presume this addresses a comment from Philip. I have no opinion
> on that. ]
Yes, I suggested this to have all user options of the package be part of
the same group.
But I think there were a few other points in my review that you should
take a look at again. I have abbreviated the diff to exclude all
non-functional changes (that I still think are worthwhile, again since
people learn Elisp from reading code):
--8<---------------cut here---------------start------------->8---
---
lisp/Makefile-agenda.make | 2 +-
lisp/casual-csv-utils.el | 2 +-
lisp/casual-dired-variables.el | 2 +-
lisp/casual-ediff-utils.el | 4 ++--
lisp/casual-editkit-utils.el | 6 +++---
lisp/casual-eshell-utils.el | 2 +-
lisp/casual-info-variables.el | 2 +-
lisp/casual.el | 4 ++--
8 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/lisp/Makefile-agenda.make b/lisp/Makefile-agenda.make
index 8f990993d0..34b785d4cf 100644
--- a/lisp/Makefile-agenda.make
+++ b/lisp/Makefile-agenda.make
@@ -22,7 +22,7 @@ casual-agenda-settings.el
ELISP_PACKAGES=
ELISP_TEST_INCLUDES=casual-agenda-test-utils.el
PACKAGE_PATHS= \
--L $(EMACS_ELPA_DIR)/compat-current \
+-L $(EMACS_ELPA_DIR)/compat-current \ #why do you test for compat
if you don't use it?
-L $(EMACS_ELPA_DIR)/seq-current \
-L $(EMACS_ELPA_DIR)/transient-current \
-L $(EMACS_ELPA_DIR)/cond-let-current \
diff --git a/lisp/casual-csv-utils.el b/lisp/casual-csv-utils.el
index 7805d575e9..6ba4384e1a 100644
--- a/lisp/casual-csv-utils.el
+++ b/lisp/casual-csv-utils.el
@@ -59,7 +59,7 @@ plain ASCII-range string."
(defun casual-csv-align-auto ()
"Auto align CSV fields."
(interactive)
- (setopt csv-align-style 'auto)
+ (setopt csv-align-style 'auto) ;why are you using `setopt' here?
(call-interactively #'csv-align-fields))
(defun casual-csv-align-left ()
diff --git a/lisp/casual-dired-variables.el b/lisp/casual-dired-variables.el
index 7b040d5f4c..85d81e2cb4 100644
--- a/lisp/casual-dired-variables.el
+++ b/lisp/casual-dired-variables.el
@@ -56,7 +56,7 @@ This variable requires GNU ‘ls’ from coreutils installed.
See the man page `ls(1)' for details."
:type '(repeat string)
- :group 'dired)
+ :group 'dired) ;Please do not add your options to
existing groups!
(define-obsolete-variable-alias 'casual-dired-use-unicode-symbols
'casual-lib-use-unicode
diff --git a/lisp/casual-ediff-utils.el b/lisp/casual-ediff-utils.el
index 543a3712e4..1ec082fea8 100644
--- a/lisp/casual-ediff-utils.el
+++ b/lisp/casual-ediff-utils.el
@@ -112,8 +112,8 @@ the current buffer."
(when (and (bound-and-true-p buffer-file-name)
(vc-registered (buffer-file-name)))
(if (and (buffer-modified-p)
- (y-or-n-p (format "Buffer %s is modified. Save buffer? "
- (buffer-name))))
+ (yes-or-no-p (format "Buffer %s is modified. Save buffer? " ; if
the user prefers y-or-no-p, the should yet use-short-answers, but otherwise you
shouldn't force short answers on them
+ (buffer-name))))
(save-buffer (current-buffer)))
(message buffer-file-name)
(casual-ediff--internal-last-revision))
diff --git a/lisp/casual-editkit-utils.el b/lisp/casual-editkit-utils.el
index a0629deba0..e92ca5ee82 100644
--- a/lisp/casual-editkit-utils.el
+++ b/lisp/casual-editkit-utils.el
@@ -63,7 +63,7 @@
(defun casual-editkit-package-symbol-overlay-installed-p ()
"Predicate to test if package `symbol-overlay' is installed."
- (package-installed-p 'symbol-overlay))
+ (package-installed-p 'symbol-overlay)) ;note that this might be an issue for
people using third-party package managers
(defun casual-editkit-package-magit-installed-p ()
"Predicate to test if package `magit' is installed."
@@ -892,7 +892,7 @@ accessed here."
(defun casual-editkit-copy-defun ()
"Copy defun point is in."
(interactive)
- (save-excursion
+ (save-excursion ;maybe `save-mark-and-excursion'?
(mark-defun)
(kill-ring-save (region-beginning) (region-end))))
@@ -1076,7 +1076,7 @@ with no space between."
"Current project label."
(let* ((project (project-current)))
(if project
- (format "Project: %s" (nth 2 project))
+ (format "Project: %s" (project-name project)) ; you are assuming that
project is always a VC project, but the project.el interface doesn't promise
this
"Project")))
(provide 'casual-editkit-utils)
diff --git a/lisp/casual-eshell-utils.el b/lisp/casual-eshell-utils.el
index 1f98ff0da5..eeca426cc0 100644
--- a/lisp/casual-eshell-utils.el
+++ b/lisp/casual-eshell-utils.el
@@ -98,7 +98,7 @@ plain ASCII-range string."
(localname (tramp-file-name-localname path-obj)))
(format "(%s) %s" host localname))
- (if (string= path (getenv "HOME"))
+ (if (file-equal-p path (getenv "HOME"))
"~"
(replace-regexp-in-string
(concat "^" (getenv "HOME")) "~" path)))))
diff --git a/lisp/casual-info-variables.el b/lisp/casual-info-variables.el
index 437a9b00a7..4743f8fc87 100644
--- a/lisp/casual-info-variables.el
+++ b/lisp/casual-info-variables.el
@@ -33,7 +33,7 @@
(defcustom casual-info-use-unicode-symbols nil
"If non-nil then use Unicode symbols whenever appropriate for labels."
:type 'boolean
- :group 'info)
+ :group 'info) ;here again, please use your
own group!
(provide 'casual-info-variables)
;;; casual-info-variables.el ends here
diff --git a/lisp/casual.el b/lisp/casual.el
index ba7c013328..8c727395ac 100644
--- a/lisp/casual.el
+++ b/lisp/casual.el
@@ -147,7 +147,7 @@ casual-re-builder, casual-lib.
Note that the package casual-lib will not be deleted if any of the packages
casual-suite, casual-avy, or casual-symbol-overlay is installed."
(interactive
- (list (y-or-n-p "Upgrade Casual to version 2?")))
+ (list (yes-or-no-p "Upgrade Casual to version 2?")))
(when enable
(let ((pkglist (list
@@ -172,7 +172,7 @@ casual-suite, casual-avy, or casual-symbol-overlay is
installed."
(package-refresh-contents)))
pkglist))))
-(defun casual-get-package-version (pkg)
+(defun casual-get-package-version (pkg) ;this is an unused,
non-interactive function?
"Get package version of symbol PKG."
(let* ((pkg-name (symbol-name pkg))
(pkg-buf (find-library pkg-name))
