* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append @ 2022-11-25 2:55 Drew Adams 2022-11-25 3:07 ` Drew Adams 0 siblings, 1 reply; 29+ messages in thread From: Drew Adams @ 2022-11-25 2:55 UTC (permalink / raw) To: 59559 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' ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2022-11-25 2:55 bug#59559: 28.1; `minibuffer-with-setup-hook' with :append Drew Adams @ 2022-11-25 3:07 ` Drew Adams 2023-01-10 17:37 ` Michael Heerdegen 0 siblings, 1 reply; 29+ messages in thread From: Drew Adams @ 2022-11-25 3:07 UTC (permalink / raw) To: Drew Adams, 59559@debbugs.gnu.org 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)). ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2022-11-25 3:07 ` Drew Adams @ 2023-01-10 17:37 ` Michael Heerdegen 2023-01-10 18:37 ` Drew Adams 0 siblings, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-10 17:37 UTC (permalink / raw) To: Drew Adams; +Cc: 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 17:37 ` Michael Heerdegen @ 2023-01-10 18:37 ` Drew Adams 2023-01-10 19:34 ` Michael Heerdegen 0 siblings, 1 reply; 29+ messages in thread From: Drew Adams @ 2023-01-10 18:37 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 18:37 ` Drew Adams @ 2023-01-10 19:34 ` Michael Heerdegen 2023-01-10 19:53 ` Michael Heerdegen 2023-01-10 20:45 ` Drew Adams 0 siblings, 2 replies; 29+ messages in thread From: Michael Heerdegen @ 2023-01-10 19:34 UTC (permalink / raw) To: Drew Adams; +Cc: 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 19:34 ` Michael Heerdegen @ 2023-01-10 19:53 ` Michael Heerdegen 2023-01-10 20:56 ` Drew Adams 2023-01-10 20:45 ` Drew Adams 1 sibling, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-10 19:53 UTC (permalink / raw) To: Drew Adams; +Cc: 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 19:53 ` Michael Heerdegen @ 2023-01-10 20:56 ` Drew Adams 0 siblings, 0 replies; 29+ messages in thread From: Drew Adams @ 2023-01-10 20:56 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559@debbugs.gnu.org > > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 19:34 ` Michael Heerdegen 2023-01-10 19:53 ` Michael Heerdegen @ 2023-01-10 20:45 ` Drew Adams 2023-01-21 14:36 ` Michael Heerdegen 1 sibling, 1 reply; 29+ messages in thread From: Drew Adams @ 2023-01-10 20:45 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559@debbugs.gnu.org > > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-10 20:45 ` Drew Adams @ 2023-01-21 14:36 ` Michael Heerdegen 2023-01-21 15:35 ` Eli Zaretskii 2023-01-21 18:41 ` Drew Adams 0 siblings, 2 replies; 29+ messages in thread From: Michael Heerdegen @ 2023-01-21 14:36 UTC (permalink / raw) To: Drew Adams; +Cc: 59559@debbugs.gnu.org [-- 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. ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 14:36 ` Michael Heerdegen @ 2023-01-21 15:35 ` Eli Zaretskii 2023-01-21 16:07 ` Michael Heerdegen 2023-01-21 18:51 ` Drew Adams 2023-01-21 18:41 ` Drew Adams 1 sibling, 2 replies; 29+ messages in thread From: Eli Zaretskii @ 2023-01-21 15:35 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559, drew.adams > 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"?). ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 15:35 ` Eli Zaretskii @ 2023-01-21 16:07 ` Michael Heerdegen 2023-01-21 18:43 ` Eli Zaretskii 2023-01-21 18:51 ` Drew Adams 1 sibling, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-21 16:07 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59559, drew.adams 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 16:07 ` Michael Heerdegen @ 2023-01-21 18:43 ` Eli Zaretskii 2023-01-21 18:57 ` Drew Adams 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2023-01-21 18:43 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559, drew.adams > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 18:43 ` Eli Zaretskii @ 2023-01-21 18:57 ` Drew Adams 2023-01-21 19:26 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Drew Adams @ 2023-01-21 18:57 UTC (permalink / raw) To: Eli Zaretskii, Michael Heerdegen; +Cc: 59559@debbugs.gnu.org > > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 18:57 ` Drew Adams @ 2023-01-21 19:26 ` Eli Zaretskii 0 siblings, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2023-01-21 19:26 UTC (permalink / raw) To: Drew Adams; +Cc: michael_heerdegen, 59559 > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 15:35 ` Eli Zaretskii 2023-01-21 16:07 ` Michael Heerdegen @ 2023-01-21 18:51 ` Drew Adams 2023-01-21 19:23 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Drew Adams @ 2023-01-21 18:51 UTC (permalink / raw) To: Eli Zaretskii, Michael Heerdegen; +Cc: 59559@debbugs.gnu.org (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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 18:51 ` Drew Adams @ 2023-01-21 19:23 ` Eli Zaretskii 2023-01-21 20:57 ` Drew Adams 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2023-01-21 19:23 UTC (permalink / raw) To: Drew Adams; +Cc: michael_heerdegen, 59559 > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 19:23 ` Eli Zaretskii @ 2023-01-21 20:57 ` Drew Adams 0 siblings, 0 replies; 29+ messages in thread From: Drew Adams @ 2023-01-21 20:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: michael_heerdegen@web.de, 59559@debbugs.gnu.org > > > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 14:36 ` Michael Heerdegen 2023-01-21 15:35 ` Eli Zaretskii @ 2023-01-21 18:41 ` Drew Adams 2023-01-21 19:29 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Drew Adams @ 2023-01-21 18:41 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559@debbugs.gnu.org > > > 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'.) ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 18:41 ` Drew Adams @ 2023-01-21 19:29 ` Eli Zaretskii 2023-01-21 20:27 ` Michael Heerdegen 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2023-01-21 19:29 UTC (permalink / raw) To: Drew Adams; +Cc: michael_heerdegen, 59559 > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 19:29 ` Eli Zaretskii @ 2023-01-21 20:27 ` Michael Heerdegen 2023-01-22 5:58 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-21 20:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59559, Drew Adams 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-21 20:27 ` Michael Heerdegen @ 2023-01-22 5:58 ` Eli Zaretskii 2023-01-22 22:10 ` Drew Adams 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2023-01-22 5:58 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 59559, drew.adams > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-22 5:58 ` Eli Zaretskii @ 2023-01-22 22:10 ` Drew Adams 2023-01-22 22:19 ` Drew Adams ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Drew Adams @ 2023-01-22 22:10 UTC (permalink / raw) To: Eli Zaretskii, Michael Heerdegen; +Cc: 59559@debbugs.gnu.org > > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-22 22:10 ` Drew Adams @ 2023-01-22 22:19 ` Drew Adams 2023-01-23 0:15 ` Michael Heerdegen 2023-01-23 12:15 ` Eli Zaretskii 2 siblings, 0 replies; 29+ messages in thread From: Drew Adams @ 2023-01-22 22:19 UTC (permalink / raw) To: Eli Zaretskii, Michael Heerdegen; +Cc: 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-22 22:10 ` Drew Adams 2023-01-22 22:19 ` Drew Adams @ 2023-01-23 0:15 ` Michael Heerdegen 2023-01-23 3:14 ` Drew Adams 2023-01-23 12:15 ` Eli Zaretskii 2 siblings, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-23 0:15 UTC (permalink / raw) To: Drew Adams; +Cc: Eli Zaretskii, 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-23 0:15 ` Michael Heerdegen @ 2023-01-23 3:14 ` Drew Adams 2023-01-23 3:21 ` Drew Adams 2023-01-23 14:11 ` Michael Heerdegen 0 siblings, 2 replies; 29+ messages in thread From: Drew Adams @ 2023-01-23 3:14 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Eli Zaretskii, 59559@debbugs.gnu.org > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-23 3:14 ` Drew Adams @ 2023-01-23 3:21 ` Drew Adams 2023-01-23 14:11 ` Michael Heerdegen 1 sibling, 0 replies; 29+ messages in thread From: Drew Adams @ 2023-01-23 3:21 UTC (permalink / raw) To: Drew Adams, Michael Heerdegen; +Cc: Eli Zaretskii, 59559@debbugs.gnu.org > I see a vague here with CL keyword args, ... ^ connection ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-23 3:14 ` Drew Adams 2023-01-23 3:21 ` Drew Adams @ 2023-01-23 14:11 ` Michael Heerdegen 2023-01-23 16:38 ` Drew Adams 1 sibling, 1 reply; 29+ messages in thread From: Michael Heerdegen @ 2023-01-23 14:11 UTC (permalink / raw) To: Drew Adams; +Cc: Eli Zaretskii, 59559@debbugs.gnu.org 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-23 14:11 ` Michael Heerdegen @ 2023-01-23 16:38 ` Drew Adams 0 siblings, 0 replies; 29+ messages in thread From: Drew Adams @ 2023-01-23 16:38 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Eli Zaretskii, 59559@debbugs.gnu.org > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#59559: 28.1; `minibuffer-with-setup-hook' with :append 2023-01-22 22:10 ` Drew Adams 2023-01-22 22:19 ` Drew Adams 2023-01-23 0:15 ` Michael Heerdegen @ 2023-01-23 12:15 ` Eli Zaretskii 2 siblings, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2023-01-23 12:15 UTC (permalink / raw) To: Drew Adams; +Cc: michael_heerdegen, 59559 > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-01-23 16:38 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-25 2:55 bug#59559: 28.1; `minibuffer-with-setup-hook' with :append Drew Adams 2022-11-25 3:07 ` Drew Adams 2023-01-10 17:37 ` Michael Heerdegen 2023-01-10 18:37 ` Drew Adams 2023-01-10 19:34 ` Michael Heerdegen 2023-01-10 19:53 ` Michael Heerdegen 2023-01-10 20:56 ` Drew Adams 2023-01-10 20:45 ` Drew Adams 2023-01-21 14:36 ` Michael Heerdegen 2023-01-21 15:35 ` Eli Zaretskii 2023-01-21 16:07 ` Michael Heerdegen 2023-01-21 18:43 ` Eli Zaretskii 2023-01-21 18:57 ` Drew Adams 2023-01-21 19:26 ` Eli Zaretskii 2023-01-21 18:51 ` Drew Adams 2023-01-21 19:23 ` Eli Zaretskii 2023-01-21 20:57 ` Drew Adams 2023-01-21 18:41 ` Drew Adams 2023-01-21 19:29 ` Eli Zaretskii 2023-01-21 20:27 ` Michael Heerdegen 2023-01-22 5:58 ` Eli Zaretskii 2023-01-22 22:10 ` Drew Adams 2023-01-22 22:19 ` Drew Adams 2023-01-23 0:15 ` Michael Heerdegen 2023-01-23 3:14 ` Drew Adams 2023-01-23 3:21 ` Drew Adams 2023-01-23 14:11 ` Michael Heerdegen 2023-01-23 16:38 ` Drew Adams 2023-01-23 12:15 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.