RE: [External] : Re: master a7c65fc666: Allow nil value for filter-buffer-substring-function

[Drew Adams]

> > Allow nil value for filter-buffer-substring-function
> >     
> > * lisp/simple.el
> > (filter-buffer-substring): Support a nil value to be
> > more resilient.
> > (filter-buffer-substring-function): Doc fix; improve
> > and update for above change.
> 
> Why?  It's been that way "forever" already and new
> code should not put a nil in there since it'll break
> uses of `add-function`.

Not following this thread, and at the risk of
being booed or ignored, I'll chime in here...

"It's been that way 'forever'" is a legitimate
(but not strong) argument.  The rest is not, IMO.

I don't think that, in general, variables whose
names end in `-function' should be disallowed a
non-function value - in particular a value of nil.

Which values such a var can take shouldn't follow
some iron-clad rule.  They should be defined case
by case.  The point of the `-function' convention
is to indicate that the value can be, often is,
or typically is, a function, not to require that
it always be a function.

The argument that allowing a nil value for such
a variable "breaks" the use of `add-function' is
no more than a re-assertion that the variable
shouldn't have a nil value.  It's not a _reason_
why it shouldn't have a nil value.  It really
just repeats the claim.

And in particular, the use of function `ignore'
is not the same thing as the use of nil to mean
"NO function" or "NOT a function" - not at all.

No good argument exists for a blanket rule that
a var whose value can be a function shouldn't
also be allowed other, non-function values - in
particular the value nil.  At least we've seen
no good argument so far.

Arguments that such a proscription lets code
blindly apply the value as a function, or blindly
advise it, are misplaced, IMO.  One use case's
"handy" is another use case's "obstacle" or
"obfuscation".

(eq var 'ignore) or (eq var #'ignore) is more,
not less, problematic than (null var).

You can certainly use (when var (add-function...))
instead of just (add-function...).  Or instead of
`apply' or `funcall'.  Not a problem -- or if you
think it is then please show the problem.

I wrote this about the same question, in thread
"[PATCH] Let bookmark handlers do everything,
including display the destination", 2022-09-05:

___

 A `nil' value does matter - it's not the same
 as calling a function unconditionally, even
 when the function is `ignore'.

 E.g., my version of `bookmark-default-handler'
 has this code, which doesn't raise the frame if
 there's no display going on (a bookmark need
 not display anything):

 (when bmkp-jump-display-function
   (save-current-buffer
     (funcall bmkp-jump-display-function (current-buffer)))
   (raise-frame))

 Sure, I could change (when XXXX ...) to
 (when (eq XXXX 'ignore) ...), but that wouldn't
 really improve anything, IMO.

 And I test the display function in some handlers,
 for conditional handling/dispatch.  E.g., my
 handler for Dired bookmarks:

 (defun bmkp-jump-dired (bookmark)
   "..."
   (let ((dir
          (bookmark-prop-get bookmark 'dired-directory))
         (mfiles
          (bookmark-prop-get bookmark 'dired-marked))
         (switches
          (bookmark-prop-get bookmark 'dired-switches))
         (subdirs
          (bookmark-prop-get bookmark 'dired-subdirs))
         (hidden-dirs
          (bookmark-prop-get bookmark 'dired-hidden-dirs)))
     (case bmkp-jump-display-function
       ((nil bmkp--pop-to-buffer-same-window display-buffer)
        (dired dir switches))
       ((bmkp-select-buffer-other-window
         pop-to-buffer
         switch-to-buffer-other-window)
        (dired-other-window dir switches))
       (t (dired dir switches)))
     (let ((inhibit-read-only  t))
       (dired-insert-old-subdirs subdirs)
       (dired-mark-remembered 
        (mapcar (lexical-let ((dd  dir))
                  (lambda (mf)
                    (cons (expand-file-name mf dd) 42)))
                mfiles))
       (save-excursion
         (dolist (dir  hidden-dirs)
           (when (dired-goto-subdir dir)
             (dired-hide-subdir 1)))))
     (let ((pos  (bookmark-get-position bookmark)))
       (when pos (goto-char pos)))))

 Obviously, testing for functional equality isn't
 possible.  But testing a function symbol lets a
 particular kind of bookmark jump differently
 depending on current context/settings.  

 > > Indeed :-(

 Not so obvious to me.  Fine as a general guideline.
 Not clear that it's a real win (no loss) here.