bug#22457: 24.5; [PATCH] `dired-mark-if' should not count non-changes

[Drew Adams <drew.adams@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]

> > Here is a fixed version of the macro.  The diff is trivial.
> 
> No diff was included, which, as usual, is a shame, because the macro has
> changed slightly in the meantime.

That makes no difference. ;-)  And the diff is still
trivial. ("The macro has changed slightly" == ?\040
became ?\s.  Nothing more)

> Your lines are also too wide; they should be less than 80 characters
> wide.

Oh, lines 92 chars are too wide for dired.el?
Then why does it have lots of lines wider than
80 chars, including lines up to 103 chars wide?

> Could you re-spin your change and submit a patch?

diff -u dired.el dired-2019-06-25a-patched.el
--- dired.el	2019-06-25 07:57:35.886354900 -0700
+++ dired-2019-06-25a-patched.el	2019-06-25 08:09:23.058466400 -0700
@@ -538,7 +538,7 @@
 ;;; Macros must be defined before they are used, for the byte compiler.
 
 (defmacro dired-mark-if (predicate msg)
-  "Mark all files for which PREDICATE evals to non-nil.
+  "Mark files for PREDICATE, according to `dired-marker-char'.
 PREDICATE is evaluated on each line, with point at beginning of line.
 MSG is a noun phrase for the type of files being marked.
 It should end with a noun that can be pluralized by adding `s'.
@@ -558,13 +558,11 @@
 		   "")))
       (goto-char (point-min))
       (while (not (eobp))
-        (if ,predicate
-            (progn
-              (delete-char 1)
-              (insert dired-marker-char)
-              (setq count (1+ count))))
+        (when ,predicate
+          (unless (looking-at-p (char-to-string dired-marker-char))
+            (delete-char 1) (insert dired-marker-char) (setq count (1+ count))))
         (forward-line 1))
-      (if ,msg (message "%s %s%s %s%s."
+      (when ,msg (message "%s %s%s %s%s"
                         count
                         ,msg
                         (dired-plural-s count)

----

Actually, the _attached_ definition is much better than
this - it's what I use in `dired+.el'.  The value returned
and the message indicate both the number matched and the
number changed.  And it has optional arg PLURAL, for
irregular plurals (e.g. "directories").

But if you want this one you'll have to do your own line
shortening, `diff', and adjustment of any callers you
might to want to pass an irregular PLURAL form.

[-- Attachment #2: throw-dired-mark-if.el --]
[-- Type: application/octet-stream, Size: 2330 bytes --]

;; 1. Value returned and message indicate both the number matched and the number changed.
;; 2. Added optional arg PLURAL, for irregular plurals (e.g. "directories").
;;
(defmacro dired-mark-if (predicate singular &optional plural)
  "Mark files for PREDICATE, according to `dired-marker-char'.
PREDICATE is evaluated on each line, with point at beginning of line.
SINGULAR is a singular noun phrase for the type of files being marked.
Optional arg PLURAL is a plural noun phrase for the type of files
 being marked.
If PLURAL is nil then SINGULAR should end with a noun that can be
pluralized by adding `s'.

Return nil if no files matched PREDICATE.
Otherwise return a cons (CHANGED . MATCHED), where:
 CHANGED is the number of markings that were changed by the operation.
 MATCHED is the number of files that matched PREDICATE."
  `(let ((inhibit-read-only  t)
         changed matched)
     (save-excursion
       (setq matched  0
             changed  0)
       (when ,singular (message "%s %s%s..."
                                (cond ((eq dired-marker-char ?\040)            "Unmarking")
                                      ((eq dired-del-marker dired-marker-char) "Flagging")
                                      (t                                       "Marking"))
                                (or ,plural  (concat ,singular "s"))
                                (if (eq dired-del-marker dired-marker-char) " for deletion" "")))
       (goto-char (point-min))
       (while (not (eobp))
         (when ,predicate
           (setq matched  (1+ matched))
           (unless (diredp-looking-at-p (char-to-string dired-marker-char))
             (delete-char 1) (insert dired-marker-char) (setq changed  (1+ changed))))
         (forward-line 1))
       (when ,singular (message "%s %s%s%s newly %s%s"
                                matched
                                (if (= matched 1) ,singular (or ,plural  (concat ,singular "s")))
                                (if (not (= matched changed)) " matched, " "")
                                (if (not (= matched changed)) changed "")
                                (if (eq dired-marker-char ?\040) "un" "")
                                (if (eq dired-marker-char dired-del-marker) "flagged" "marked"))))
     (and (> matched 0)  (cons changed matched))))