bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

Starting from `emacs -Q', try to use `minibuffer-with-setup-hook' with a
list as its arg FUN, where the car is :append and the cadr is a function.
I get an error no matter how I try to interpret the description of the
FUN arg with :append.

The code for the macro does seem to try to handle :append, to construct
code that calls `add-hook' with a non-nil APPEND arg.  But `macroexpand'
with any such list arg to `minibuffer-with-setup-hook' (:append FUNCTION)
doesn't seem to construct an `add-hook' sexp with an APPEND arg.

I see this in every Emacs version that has `minibuffer-with-setup-hook'.
Maybe I'm just not understanding the doc string.  What's an example of
using `minibuffer-with-setup-hook' with (:append FUNCTION), that results
in FUNCTION being appended to `minibuffer-setup-hook', instead of
raising an error?

In GNU Emacs 28.1 (build 2, x86_64-w64-mingw32)
 of 2022-04-21 built on AVALON
Windowing system distributor 'Microsoft Corp.', version 10.0.19044
System Description: Microsoft Windows 10 Pro (v10.0.2009.19044.2251)

Configured using:
 'configure --with-modules --without-dbus --with-native-compilation
 --without-compress-install CFLAGS=-O2'

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

Note that the macroexpansion is different in different
Emacs releases.  But none of them look right to me.  They
essentially produce some variant of (funcall (:append FUN)).

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> Note that the macroexpansion is different in different
> Emacs releases.  But none of them look right to me.  They
> essentially produce some variant of (funcall (:append FUN)).

In master I get

#+begin_src emacs-lisp
(macroexpand-1
  '(minibuffer-with-setup-hook (:append test1) (test2)))
==>
 (let
    ((#2=#:fun test1)
     (#1=#:setup-hook
      (make-symbol "minibuffer-setup")))
  (fset #1#
        (lambda nil
          (remove-hook 'minibuffer-setup-hook #1#)
          (funcall #2#)))
  (unwind-protect
      (progn
        (add-hook 'minibuffer-setup-hook #1# t)
        (test2))
    (remove-hook 'minibuffer-setup-hook #1#)))
#+end_src

At the first look that does not look wrong.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

I see that too.  Dunno what problem I was seeing.
I suspect I was passing '(:append foo) instead of
(:append foo).  I guess this can be closed.

Maybe it would be clearer if the doc string said
that FUN is not evaluated but BODY is?  Yes, it
does say "while executing BODY", but maybe it's
not so obvious that FUN isn't evaluated?  Dunno.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> Maybe it would be clearer if the doc string said
> that FUN is not evaluated but BODY is?  Yes, it
> does say "while executing BODY", but maybe it's
> not so obvious that FUN isn't evaluated?  Dunno.

AFAIU FUN is either a function expression or an (unquoted) list
(:append FUN) where FUN is a function expression.

With other words, the function part is evaluated, but the list
(:append FUN) should not be quoted (it's not evaluated).

I think clarifying that a bit would make sense.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> AFAIU FUN is either a function expression or an (unquoted) list
> (:append FUN) where FUN is a function expression.
                               ^^^^^^^^^^^^^^^^^^^

Correction: AFAIR we use that term for "lambda expression or function
name".  Here any expression is allowed as long as it returns a function,
so I would just say "expression".

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > Maybe it would be clearer if the doc string said
> > that FUN is not evaluated but BODY is?  Yes, it
> > does say "while executing BODY", but maybe it's
> > not so obvious that FUN isn't evaluated?  Dunno.
> 
> AFAIU FUN is either a function expression or an (unquoted) list
> (:append FUN) where FUN is a function expression.
> 
> With other words, the function part is evaluated, but the list
> (:append FUN) should not be quoted (it's not evaluated).

Yes, maybe that was it.  I should have included
more info, e.g., an example to repro what I
described.

> I think clarifying that a bit would make sense.

Yes.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > AFAIU FUN is either a function expression or an (unquoted) list
> > (:append FUN) where FUN is a function expression.
>                                ^^^^^^^^^^^^^^^^^^^
> 
> Correction: AFAIR we use that term for "lambda expression or function
> name".  Here any expression is allowed as long as it returns a function,
> so I would just say "expression".

It's maybe not obvious whether, when we use
"expression" we're suggesting that that gets
_evaluated_.

What needs to be made clear here, I think, is
that in (:append F) F is evaluated, and its
value should be a function.  And that function
is invoked (funcalled) with no args. 

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

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

Drew Adams <drew.adams@oracle.com> writes:

> > I think clarifying that a bit would make sense.
>
> Yes.

I tried to do that:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-files.el-minibuffer-with-setup-hook-Clarify-doc.patch --]
[-- Type: text/x-diff, Size: 1390 bytes --]

From 18a09740e4ee354954803011a7e7ded003987f77 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sat, 21 Jan 2023 14:45:39 +0100
Subject: [PATCH] * lisp/files.el (minibuffer-with-setup-hook): Clarify
 docstring

This fixes Bug#59559: Try to make clearer what kinds of s-exps are
expected as FUN argument and how they are interpreted.
---
 lisp/files.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index d308e99804d..1da77159217 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1746,9 +1746,11 @@ confirm-nonexistent-file-or-buffer
 (defmacro minibuffer-with-setup-hook (fun &rest body)
   "Temporarily add FUN to `minibuffer-setup-hook' while executing BODY.

-By default, FUN is prepended to `minibuffer-setup-hook'.  But if FUN is of
-the form `(:append FUN1)', FUN1 will be appended to `minibuffer-setup-hook'
-instead of prepending it.
+In the default case, FUN is an expression that should evaluate to
+a function, and the result will be prepended to
+`minibuffer-setup-hook'.  If FUN is an unquoted list of the
+form `(:append FUN1)', the result of evaluating FUN1 will be
+appended to `minibuffer-setup-hook' instead of prepending it.

 BODY should use the minibuffer at most once.
 Recursive uses of the minibuffer are unaffected (FUN is not
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 55 bytes --]


I think this could go to emacs-29.

Thanks,

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> Cc: "59559@debbugs.gnu.org" <59559@debbugs.gnu.org>
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Sat, 21 Jan 2023 15:36:04 +0100
> 
>  (defmacro minibuffer-with-setup-hook (fun &rest body)
>    "Temporarily add FUN to `minibuffer-setup-hook' while executing BODY.
> 
> -By default, FUN is prepended to `minibuffer-setup-hook'.  But if FUN is of
> -the form `(:append FUN1)', FUN1 will be appended to `minibuffer-setup-hook'
> -instead of prepending it.
> +In the default case, FUN is an expression that should evaluate to
> +a function, and the result will be prepended to
> +`minibuffer-setup-hook'.  If FUN is an unquoted list of the
> +form `(:append FUN1)', the result of evaluating FUN1 will be
> +appended to `minibuffer-setup-hook' instead of prepending it.

How about just adding the obvious to the first sentence:

  Temporarily add function FUN to `minibuffer-setup-hook' while executing BODY.

All the rest sounds clear to me, and I find the original text less
confusing than your proposed change ("expression that should evaluate
to a function"?).

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> How about just adding the obvious to the first sentence:
>
>   Temporarily add function FUN to `minibuffer-setup-hook' while
> executing BODY.
>
> All the rest sounds clear to me, and I find the original text less
> confusing than your proposed change ("expression that should evaluate
> to a function"?).

The (valid, IMO) request was about the (:append FUN1) part: is this the
s-expr to specify, or should the FUN arg evaluate to such a list?  Not
clear from the original docstring.

I'm happy if you or Drew want to suggest a better wording.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > > I think clarifying that a bit would make sense.
> > Yes.
> I tried to do that:
<proposed>
  In the default case, FUN is an expression that should
  evaluate to a function, and the result will be prepended to
  `minibuffer-setup-hook'.  If FUN is an unquoted list of the
  form `(:append FUN1)', the result of evaluating FUN1 will be
  appended to `minibuffer-setup-hook' instead of prepending it.

Thx.

I'd say (but Eli will likely disagree), that we need
not and should not use future tense ("will be"),
and we should avoid "should".  But that's just a
doc-style question.

More importantly, I'd avoid talking about an unquoted
list.  It's not really about quotation, is it?  It's
about evaluation.  It's about the first arg being a
sexp that doesn't simply get evaluated.

The important thing, I think, is to get across the
(quite unusual) treatment of arg FUN: It's not just
evaluated, and it's not just NOT evaluated.  (IMO,
this is a poor interface, but we are where we are.)

I suggest something like this - somehow get across
the fact that FUN _might_ be simply evaluated, and
the result prepended, or it might be a sexp that's
_not_ evaluated, but _part_ of it is, and in that
case the result of that evaluation is appended.

E.g.:

  Argument FUN is a sexp; it is not simply evaluated.
  Two cases:
  * If FUN has form (:append FUNCTION), evaluate
    FUNCTION and append the result to the hook.
  * Otherwise, evaluate FUN and prepend the result
    to the hook.

Another possibility:

  If FUN is (without evaluating) a sexp (:append FUNCTION)
  then FUNCTION is evaluated and appended to the hook.
  Otherwise, FUN is evaluated and prepended to the hook.

(Could say "list" instead of "sexp", but the latter
stresses the connotation that it's not evaluated.)

We might also change the name FUN.  Maybe use FUN-SPEC
or something - something that doesn't suggest that it
_is_ a function but that it specifies a function that
gets added to the hook.

A question is whether what is said in the doc of
`add-hook' is also true here: the function "is not
added if already present."

If so, I think the doc should say that.  (It's kind
of a shame that we can't just point to `add-hook'.)

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: drew.adams@oracle.com,  59559@debbugs.gnu.org
> Date: Sat, 21 Jan 2023 17:07:18 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > How about just adding the obvious to the first sentence:
> >
> >   Temporarily add function FUN to `minibuffer-setup-hook' while
> > executing BODY.
> >
> > All the rest sounds clear to me, and I find the original text less
> > confusing than your proposed change ("expression that should evaluate
> > to a function"?).
> 
> The (valid, IMO) request was about the (:append FUN1) part: is this the
> s-expr to specify, or should the FUN arg evaluate to such a list?  Not
> clear from the original docstring.

I don't see why.  We say stuff like "argument of the form (FOO BAR)"
in gazillion places, and I see no reason why this one place should be
singled out.

> I'm happy if you or Drew want to suggest a better wording.

I already did.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

(Didn't see your mail before I replied to Michael's.)

> How about just adding the obvious to the first sentence:
> 
>   Temporarily add function FUN to `minibuffer-setup-hook' while executing
>   BODY.

To me, that doesn't help at all.  And as you would
(usually) say, "function FUN" can just be replaced
by "FUNCTION" there.

Just saying that the first arg is a function doesn't
solve the problem.  What is the first arg, exactly?
Is it evaluated?

It's a sexp.  Either that sexp is a list `(:append F)',
in which case only F is evaluated, to provide the
function to add (append), or the entire FUN sexp is
evaluated to provide the function to add (prepend).

It's this unusual behavior that needs to be understood,
and thus described - in particular pointing out that
the arg isn't just evaluated to begin with.

> All the rest sounds clear to me, and I find the original text less
> confusing than your proposed change ("expression that should evaluate
> to a function"?).

Please see the text suggestions I proposed.  Somehow
we need to get across the unusual treatment of the
first arg.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > The (valid, IMO) request was about the (:append FUN1) part: is this the
> > s-expr to specify, or should the FUN arg evaluate to such a list?  Not
> > clear from the original docstring.
> 
> I don't see why.  We say stuff like "argument of the form (FOO BAR)"
> in gazillion places, 

Precisely.  And we don't say that here.  See my
suggestion that says exactly that.  That's _one_
of the possible forms of the argument sexp.  IF
it has that form (:append F) THEN F is evaluated
to give the function that's added.  OTHERWISE
the whole sexp arg is evaluated to give the
function that's added.

This kind of thing is _not_ done in a gazillion
places.

> and I see no reason why this one place should be
> singled out.

The behavior is quite unusual; that's why.

> > I'm happy if you or Drew want to suggest a better wording.
> 
> I already did.

I don't think what you suggested changes/adds anything.
You just changed "FUN" to "function FUN", no?  The
problem isn't understanding that arg as providing the
function to add to the hook.  The problem is to get
that FUN isn't simply evaluated and it isn't simply
_not_ evaluated.  It's either a sexp of a certain form
or it's not, and the treatment of it is different in 
those two cases.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "59559@debbugs.gnu.org" <59559@debbugs.gnu.org>
> Date: Sat, 21 Jan 2023 18:51:25 +0000
> 
> > How about just adding the obvious to the first sentence:
> > 
> >   Temporarily add function FUN to `minibuffer-setup-hook' while executing
> >   BODY.
> 
> To me, that doesn't help at all.  And as you would
> (usually) say, "function FUN" can just be replaced
> by "FUNCTION" there.

That's a tangent.  Let's keep our focus where it belongs.

> Just saying that the first arg is a function doesn't
> solve the problem.  What is the first arg, exactly?
> Is it evaluated?

How is this different from the below:

  expand-file-name is a built-in function in ‘C source code’.

  (expand-file-name NAME &optional DEFAULT-DIRECTORY)

  Convert filename NAME to absolute, and canonicalize it.
  [...]
  NAME should be a string [...]

Since when do we ask about function's arguments whether they are
evaluated or not? and why for that particular function and not for
others?

> It's a sexp.  Either that sexp is a list `(:append F)',
> in which case only F is evaluated, to provide the
> function to add (append), or the entire FUN sexp is
> evaluated to provide the function to add (prepend).

You are splitting hair.  Once again, saying that an argument can (or
should) be of some form is a paradigm we use a lot in our doc strings,
and this case is not different.

> It's this unusual behavior that needs to be understood,
> and thus described - in particular pointing out that
> the arg isn't just evaluated to begin with.

No, it isn't unusual.

> > All the rest sounds clear to me, and I find the original text less
> > confusing than your proposed change ("expression that should evaluate
> > to a function"?).
> 
> Please see the text suggestions I proposed.  Somehow
> we need to get across the unusual treatment of the
> first arg.

It isn't unusual.  I see nothing here that needs some special wording.

And let's not make this another endless discussion where you refuse to
accept the judgment of others.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "59559@debbugs.gnu.org" <59559@debbugs.gnu.org>
> Date: Sat, 21 Jan 2023 18:57:45 +0000
> 
> > > The (valid, IMO) request was about the (:append FUN1) part: is this the
> > > s-expr to specify, or should the FUN arg evaluate to such a list?  Not
> > > clear from the original docstring.
> > 
> > I don't see why.  We say stuff like "argument of the form (FOO BAR)"
> > in gazillion places, 
> 
> Precisely.  And we don't say that here.

Yes, we do:

  But if FUN is of the form ‘(:append FUN1)’ [...]

> That's _one_
> of the possible forms of the argument sexp.  IF
> it has that form (:append F) THEN F is evaluated
> to give the function that's added.  OTHERWISE
> the whole sexp arg is evaluated to give the
> function that's added.
> 
> This kind of thing is _not_ done in a gazillion
> places.

It's a macro.  That's how macros are handled in Emacs.

> > I already did.
> 
> I don't think what you suggested changes/adds anything.
> You just changed "FUN" to "function FUN", no?

Yes, because that's the only part that is not crystal clear there.

I don't see what else needs to be discussed here.  We should close
this bug.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> Cc: "59559@debbugs.gnu.org" <59559@debbugs.gnu.org>
> From: Drew Adams <drew.adams@oracle.com>
> Date: Sat, 21 Jan 2023 18:41:15 +0000
> 
> I suggest something like this - somehow get across
> the fact that FUN _might_ be simply evaluated, and
> the result prepended, or it might be a sexp that's
> _not_ evaluated, but _part_ of it is, and in that
> case the result of that evaluation is appended.
> 
> E.g.:
> 
>   Argument FUN is a sexp; it is not simply evaluated.
>   Two cases:
>   * If FUN has form (:append FUNCTION), evaluate
>     FUNCTION and append the result to the hook.
>   * Otherwise, evaluate FUN and prepend the result
>     to the hook.
> 
> Another possibility:
> 
>   If FUN is (without evaluating) a sexp (:append FUNCTION)
>   then FUNCTION is evaluated and appended to the hook.
>   Otherwise, FUN is evaluated and prepended to the hook.
> 
> (Could say "list" instead of "sexp", but the latter
> stresses the connotation that it's not evaluated.)
> 
> We might also change the name FUN.  Maybe use FUN-SPEC
> or something - something that doesn't suggest that it
> _is_ a function but that it specifies a function that
> gets added to the hook.

All of your suggestions just muddy the water, making a clear doc
string confusing and risking the user's interpreting that like some
kind of black magic is going on here.

The current doc string is clear and easy to understand and use.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> All of your suggestions just muddy the water, making a clear doc
> string confusing and risking the user's interpreting that like some
> kind of black magic is going on here.
>
> The current doc string is clear and easy to understand and use.

I don't think we will find a version that makes us all happy.  I was
actually fine with the original version:

While the syntax of this macro is a bit unusual, with the background of
the terminology used in the rest of Emacs the description allows only
one interpretation: when we say that the argument (of a macro) is of a
certain form, the unevaluated s-exp is meant.  Else we explicitly say
that the value or evaluation result should have certain properties.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > > How about just adding the obvious to the first sentence:
> > >
> > >   Temporarily add function FUN to `minibuffer-setup-hook' while
> > >   executing BODY.
> >
> > To me, that doesn't help at all.  And as you would
> > (usually) say, "function FUN" can just be replaced
> > by "FUNCTION" there.
> 
> That's a tangent.  Let's keep our focus where it belongs.

Precisely.  That was all that you suggested: add
the word "funtion" - no?  Instead of focusing on
the proble/bug report, your "just add" suggestion
is essentially tangent.

> > Just saying that the first arg is a function doesn't
> > solve the problem.  What is the first arg, exactly?
> > Is it evaluated?
> 
> How is this different from the below:
> 
>   expand-file-name is a built-in function in ‘C source code’.
>   (expand-file-name NAME &optional DEFAULT-DIRECTORY)
>   Convert filename NAME to absolute, and canonicalize it.
>   [...]
>   NAME should be a string [...]
> 
> Since when do we ask about function's arguments whether they are
> evaluated or not? and why for that particular function and not for
> others?

`expand-file-name' is a function, not a macro.
_Of course_ it evaluates its args.  And saying
that NAME is a file name, and that it's a string,
is 100% correct and adequate.

`minibuffer-with-setup-hook' is a macro.  And
it's not the case that all its args are eval'd
or not eval'd.  More importantly, it's not even
the case that its first arg is simply eval'd or
simply not eval'd.  The first arg is treated
quite unusually - if it's one kind of sexp then
part of it is eval'd; if it's not that kind of
sexp then the whole arg is eval'd.  There's no
way to guess this behavior.  The doc needs to
help, here.

> > It's a sexp.  Either that sexp is a list `(:append F)',
> > in which case only F is evaluated, to provide the
> > function to add (append), or the entire FUN sexp is
> > evaluated to provide the function to add (prepend).
> 
> You are splitting hair.  Once again, saying that an argument can (or
> should) be of some form is a paradigm we use a lot in our doc strings,
> and this case is not different.

See above.  It's not just about the form of the
sexp.  It's about the fact that it's a sexp,
and how it's handled being different in the two
cases.  It's not even obvious that the arg isn't
simply evaluated always, although one might
guess something more is going on if one notices
that it's a macro.

> > It's this unusual behavior that needs to be understood,
> > and thus described - in particular pointing out that
> > the arg isn't just evaluated to begin with.
> 
> No, it isn't unusual.

Please point to some other macros that treat
the same argument differently wrt evaluation.

No doubt there are a few.  But I expect that
those generally have multiple forms of the
sexp arg, and the doc catalogs the behavior
of each form, clearly.

> > > All the rest sounds clear to me, and I find the original text less
> > > confusing than your proposed change ("expression that should evaluate
> > > to a function"?).
> >
> > Please see the text suggestions I proposed.  Somehow
> > we need to get across the unusual treatment of the
> > first arg.
> 
> It isn't unusual.  I see nothing here that needs some special wording.
> 
> And let's not make this another endless discussion 
> where you refuse to accept the judgment of others.

(Here you go again, with the ad hominem.  You
never fail to make it about me, do you?  Think
about it, please.)

We always accept the fact that your judgment
is final.  That doesn't mean your judgment is
always sound or best, of course - we're all
only human.  But yes, you're the decider (and
I, for one, am glad of that - as I've said
many times).

_Did_ you make a final judgment here?  I
thought suggestions for the doc were still
being made.  I thought efforts were being made
to find a doc string that makes clear what the
second arg needs to be and how it's handled.

Maybe consider making your decisions more
obviously decisions, instead of speaking of
your contributions as "suggestions I proposed".
Otherwise, it's too easy to take you at your
word as a participating peer with some helpful
suggestions.

Close the bug, if you must.  I take no pleasure
in having to repeatedly explain what the problem
is.  You don't see the problem - fine; so be it.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Drew Adams <drew.adams@oracle.com>,  59559@debbugs.gnu.org
> Date: Sat, 21 Jan 2023 21:27:34 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > All of your suggestions just muddy the water, making a clear doc
> > string confusing and risking the user's interpreting that like some
> > kind of black magic is going on here.
> >
> > The current doc string is clear and easy to understand and use.
> 
> I don't think we will find a version that makes us all happy.  I was
> actually fine with the original version:
> 
> While the syntax of this macro is a bit unusual, with the background of
> the terminology used in the rest of Emacs the description allows only
> one interpretation: when we say that the argument (of a macro) is of a
> certain form, the unevaluated s-exp is meant.

Full agreement, my thoughts exactly.

> Else we explicitly say that the value or evaluation result should
> have certain properties.

That would require us to write a much more complex doc string in this
case, and then do the same for all the other macros we have.  So I
don't think this is a good idea.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> > While the syntax of this macro is a bit unusual, with the background of
> > the terminology used in the rest of Emacs the description allows only
> > one interpretation: when we say that the argument (of a macro) is of a
> > certain form, the unevaluated s-exp is meant.

But that's _not_ what's meant here.  The doc string:

  By default, FUN is prepended to 'minibuffer-setup-hook'.
  But if FUN is of the form '(:append FUN1)', FUN1 will
  be appended to 'minibuffer-setup-hook' instead of prepending it.

Whether FUN is or is not of that :append form, per
your presumption - "the terminology used in the rest
of Emacs", the UNevaluated sexp is meant.  But that's
not the case here.

Try it:

(defun toto () (message "@@@@@@@@@@@@@@"))
(minibuffer-with-setup-hook toto (message "************"))

Debugger entered--Lisp error: (void-variable toto)
  (let ((fun toto)
    (setup-hook (make-symbol "minibuffer-setup")))
    (fset setup-hook
          #'(lambda nil
              (remove-hook 'minibuffer-setup-hook setup-hook)
              (funcall fun)))
    (unwind-protect (progn (add-hook 'minibuffer-setup-hook setup-hook)
                           (message "************"))
      (remove-hook 'minibuffer-setup-hook setup-hook)))

Not to mention that the unevaluated form (:append toto)
also exhibits the same problem - it too doesn't fit
"the terminology used in the rest of Emacs".

It is just not the case that "the unevaluated s-exp is
meant" for macro `minibuffer-with-setup-hook'.  The
_evaluated_ sexp is meant for FUN as the arg, and the
_evaluated_ FUN1 _part_ of the sexp (:append FUN1) is
meant for that form of the arg.

That's what this bug report is about.  And yes, this
_is_ quite unusual.  IMO it's not the best interface
design, but there's nothing really "wrong" with
defining a macro whose argument handling is this
complicated.

There is something wrong, however, with not making
this unusual behavior clear in the macro's doc, IMHO.

> Full agreement, my thoughts exactly.
> 
> > Else we explicitly say that the value or 
> > evaluation result should have certain properties.
> 
> That would require us to write a much more complex doc string in this
> case, and then do the same for all the other macros we have.  So I
> don't think this is a good idea.

The "much more complex doc string", which is not much
more complex IMO, is needed because the behavior of
the macro itself is complex - it is _not_ the simple
behavior you've both said it is.

It's not the usual case for a macro (that the args
aren't evaluated).  And it's not the also usual case
that some of the args are evaluated but others are not.

It's the unusual case that one of the args is eval'd
if it has one form, and only _part_ of that arg is
eval'd and used if it has another form.

I say all this because it seems like you two are still
discussing this.  But if Eli has reached a decision
that there's no bug (have you, Eli?) then end of story.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

BTW, 90% (18/20) uses of `minibuffer-with-setup-hook'
in the Emacs (26.3) sources use a lambda form as FUN.

And of course a lambda form evaluates to itself.
And one of those lambda forms is quoted with #'.
And the two non-lambda FUN occurrences are each
quoted with #'.  Clearly, FUN is evaluated.

And there are NO uses of the macro with (:append FN).

So it's maybe not too surprising that no one has
reported this doc bug previously.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> Whether FUN is or is not of that :append form, per
> your presumption - "the terminology used in the rest
> of Emacs", the UNevaluated sexp is meant.  But that's
> not the case here.
>
> Try it:
>
> (defun toto () (message "@@@@@@@@@@@@@@"))
> (minibuffer-with-setup-hook toto (message "************"))
>
> Debugger entered--Lisp error: (void-variable toto)
>   (let ((fun toto)
>     (setup-hook (make-symbol "minibuffer-setup")))
>     (fset setup-hook
>           #'(lambda nil
>               (remove-hook 'minibuffer-setup-hook setup-hook)
>               (funcall fun)))
>     (unwind-protect (progn (add-hook 'minibuffer-setup-hook setup-hook)
>                            (message "************"))
>       (remove-hook 'minibuffer-setup-hook setup-hook)))
>
> Not to mention that the unevaluated form (:append toto)
> also exhibits the same problem - it too doesn't fit
> "the terminology used in the rest of Emacs".

Well yes, I know, I expected that behavior after reading the docstring.
You obviously don't.

Nowhere is said that FUN should be a function _name_ or a symbol.

Unless stated otherwise, what you specify as an argument is an
expression.  Here only with the exception that two syntaxes are
possible, the alternative one is a list, like we e.g. know from cl
arglists.

My patch tried to make that part clearer: the argument is of the form
FUN or (:append FUN) (an macro _argument_ described as of a certain form
always speaks about the unevaluated s-exp), and FUN is an expression
evaluating to the function that will be added to the hook.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> Well yes, I know, I expected that behavior after reading
> the docstring.  You obviously don't.
> 
> Nowhere is said that FUN should be a function _name_ or a symbol.

If it's not evaluated, and we say that it's added to a
hook, then it must be a sexp that represents a function
in some way, no?  What kinds of sexps represent/name
functions?  Lambda forms and defined-function names
(symbols).  (And maybe someone could think the symbol
name does also.)

> Unless stated otherwise, what you specify as an argument
> is an expression.

But given that assumption, what expressions are expected
for argument FUN?

The doc says that FUN is added to the hook.  Only functions
are added to a hook.  So FUN, the sexp, must be a function?
A sexp isn't a function, and a function isn't a sexp.

So one guesses that maybe it means a sexp that _represents_
a function.  OK. What kinds of specs represent functions?

You see where this leads, I hope.  Is that sexp evaluated?
Do I quote a function name or just pass the function name
(which certainly represents/names a function)?  That info
is missing.  It's not enough to know that macro args are
sexps.

And then we get to the part about the other, more particular
form of spec you can use for FUN - the other sexp that
represents a function, (:append FUN1).

Here we're told not that the whole thing (which is FUN from
the signature and the first line) is added to the hook, but
that FUN1 is added to the hook.

So already there's a contradiction from the the first line.
Even so, now we have that the FUN1 sexp part of the sexp
(:append FUN1) is actually what gets added.  OK.

And we're back to the same question as for FUN: a sexp that
represents a function.  Is it eval'd?  Quoted or unquoted
function representation?   Doesn't matter for a lambda, but
does matter for a function name.

Note that the doc string even goes to the trouble of adding
the detail that "This macro actually adds an auxiliary
function that calls FUN, rather than FUN itself, to
'minibuffer-setup-hook'."  Is that so important for users?

And yet we don't want to make clear whether the sexps (FUN
and FUN1) that represent functions that get added (oops - in
an auxiliary function) get evaluated, i.e., whether you pass
foobar or #'foobar?  Why isn't that important to communicate?

The doc isn't clear, IMO, and it doesn't help to say that if
a reader just understands that macros don't eval their args
then it should be clear.

> Here only with the exception that two syntaxes are
> possible, the alternative one is a list, like we e.g. 
> know from cl arglists.

I see a vague here with CL keyword args, if that's what
you're suggesting.  But CL doc would never show the lambda
list in a way that doesn't make a keyword arg clear, and it
would specifically call out the use of the supported keyword
arg.  (And of course, Elisp doesn't have or allow keyword
args in a lambda list.)

> My patch tried to make that part clearer: the argument is of the form
> FUN or (:append FUN) (an macro _argument_ described as of a certain form
> always speaks about the unevaluated s-exp), and FUN is an expression
> evaluating to the function that will be added to the hook.

Yes, your text was a big improvement, and yes, you tried to
address the problems I raised.  My only comment was a minor
one: to avoid talking about an "unquoted list".

And I pointed to another problem: is it true, as it is for
`add-hook', that the function "is not added if already
present"?  If so, the doc should say that.  A reader can't
assume that that's what happens - there's no mention that
`add-hook' is involved, for instance.  Neither of you have
spoken to this question.

I think the doc should make clear (explicit) that there're
two _alternative_ FUNC arg forms, that both are sexps, and
that in one form the entire sexp is eval'd to produce the
function that's added, and in the (:append F) form the F
part is eval'd to give the function that's added.

Also, I don't think that the non-(:append F) form is any
more of a "default" than the (:append F) form.  There are
two acceptable forms for that sexp arg; neither is default.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> I see a vague here with CL keyword args, ...
               ^
           connection

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "59559@debbugs.gnu.org" <59559@debbugs.gnu.org>
> Date: Sun, 22 Jan 2023 22:10:49 +0000
> 
> But if Eli has reached a decision

He did.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Michael Heerdegen]

Drew,

thanks for your elaborations.

I don't have a strong opinion.  I can follow your thoughts, but I'm also
not really convinced that writing that all out in the docstring is
necessary.

So I would like to leave it up to you two to find a final agreement.

Michael.

bug#59559: 28.1; `minibuffer-with-setup-hook' with :append

[Drew Adams]

> Drew, thanks for your elaborations.

You're welcome.  Thanks for working on this.

> I don't have a strong opinion.  I can follow your thoughts, but I'm also
> not really convinced that writing that all out in the docstring is
> necessary.
> 
> So I would like to leave it up to you two to find a final agreement.

Eli has now decided - apparently it's "won't fix".

FWIW, I think this kind of interface is inherently
problematic (for the reasons I gave).  It's not
similar to, say, `add-hook', where APPEND can be
an optional arg.  (Nor is it like CL keyword args.)

No one has any trouble understanding that a macro
like `with-current-buffer' evals its first arg,
so no special need for the doc to point that out.
If this case were like that (and others, similar)
I wouldn't have filed the bug report.

It's the use by `minibuffer-with-setup-hook' of
the same arg with two forms, and with evaluation
not always of that arg but sometimes of just part
of it -- that's what makes it problematic for the
doc to just talk about FUN being a function.  IMO
the doc does _need_ to talk about how the arg is
handled (evaluated).

FWIW, I would have just had two macros, instead
of fiddling with a special kind of arg:

1. `with-mbuf-setup-hook-add'  (or
   `with-mbuf-setup-hook-prepend')

2. `with-mbuf-setup-hook-append'


That also uses fewer chars:

(with-mbuf-setup-hook-append #'toto ...)

(minibuffer-with-setup-hook (:append #'toto) ...)


Even if "minibuffer" is spelled out it's shorter:

(with-minibuffer-setup-hook-append #'toto ...)

And starting with "with-" fits what we do with
other, similar macros.