Re: Info-mode patch

[Juri Linkov <juri@linkov.net>]

>>>>>>>> But it seems this is not enough because with-selected-frame
>>>>>>>> still fails to switch focus to another frame.  You need also
>>>>>>>> to use select-frame-set-input-focus.
>>>>>>> Where it fails? For me it prompts me on correct frame. I didn't want to switch
>>>>>>> focus on the info frame. I am aware of select-frame-set-input-focus, have used
>>>>>>> it in some test actually.
>>>>>>
>>>>>> Probably the behaviour depends on the window manager.
>>>>>
>>>>> Yes, I am quite sure it isn't "probably" but "surely" :), as I wrote earlier by
>>>>> the way.
>>>>>
>>>>>> With my window manager with-selected-frame displays
>>>>>> the prompt in another frame, but input is inserted
>>>>>> into the original buffer.  Maybe we should have
>>>>>> a new option whether to use select-frame-set-input-focus?
>>>>>
>>>>> I am not sure I understand what you mean with input being inserted in the
>>>>> original buffer.
>>>>
>>>> For example, the original buffer is *scratch*.  The minibuffer
>>>> pops up in another frame.  I start typing.  But letters get
>>>> inserted to *scratch*, not to the minibuffer.  This is because
>>>> focus is not switched to another frame.
>>>
>>> Aha, ok, I understand now. I haven't checked that one, tbh. When I realized the
>>> minibufer pops on other frame, I realized I didn't liked it, so I reworked
>>> everything so focus stays on the frame where I type. As I remember now, I
>>> also have set Emacs option to auto select window based on the cursor,
>>> mouse-autoselect-window, so it might be that one that interfers too. I totally
>>> forgott that one earlier.
>>
>> I tried to set mouse-autoselect-window, but still input focus
>> is not switched to another frame with e.g.
>>
>>   (with-selected-frame (next-frame) (read-string "Prompt: "))
>
> Yes with-selected-frame does switch focus to a freame; I have also used it in
> the patch, for the same purpose to switch focus back to user frame.

Well, it works for you.  I don't know why it's not supported for any WM.

>>>>>>> Have you tested *everything*? Interactively and from lisp?
>>>>>>
>>>>>> I see no problems with this both interactively and from lisp:
>>>>>
>>>>> If you don't care to ask the user which window to choose when ambigous, then you
>>>>> don't have to care about this at all. If you don't want to take care of
>>>>> multiple windows with possibly ambigous names, user misstyping a name and
>>>>> possibly irregular name, then you don't need to prompt the user at all, just
>>>>> take the first info buffer you find or force user to *always* select manually a
>>>>> window and you are all good. But in my opinion it is not hard to have it slightly
>>>>> more polished and automated as I did.
>>>>
>>>> I don't know what do you mean by the words "don't care"
>>>> since this implementation still uses your function
>>>> 'info-window' that asks the user which window to choose.
>>>> Please look carefully at Info-index-other-window:
>>>>
>>>>>> #+begin_src emacs-lisp
>>>>>> (defmacro with-selected-window-frame (window &rest body)
>>>>>>   `(let ((old-frame (selected-frame))
>>>>>>          (frame (window-frame ,window)))
>>>>>>      (unless (eq frame old-frame)
>>>>>>        (select-frame frame 'norecord)
>>>>>>        (select-frame-set-input-focus frame 'norecord))
>>>>>>      (prog1 (with-selected-window ,window
>>>>>>               ,@body)
>>>>>>        (select-frame old-frame 'norecord)
>>>>>>        (select-frame-set-input-focus old-frame 'norecord))))
>>>>>>
>>>>>> (defun Info-index-other-window (topic &optional window)
>>>>>>   (interactive
>>>>>>    (with-selected-window-frame (info-window)
>>>>>>      (append (eval (cadr (interactive-form 'Info-index)))
>>>>>>              (list (selected-window)))))
>>>>>>   (with-selected-window (or window (info-window))
>>>>>>     (Info-index topic)))
>>>>>> #+end_src
>>>
>>> I thought it was just left over while testing. Actually you want to keep the
>>> part I disslike most, and want to redo the part(s) I think are good :). When I
>>> wrote about the patch, I had three sections of text: the good, the bad and the
>>> ugly. Part about info-window and find-window-for-help was "the ugly" part. The
>>> bad one was about alterered signatures and the good one was about everythign
>>> been possible to implement user-friendly.
>>
>> Please explain what is ugly in info-window.  I can see that in
>> find-window-for-help the name is ugly.  Indeed, there is nothing
>> specific to help in find-window-for-help.
>
> Separating funcitonality in two places. Part of required functionality is in
> that one, and part is in info/help-window. In my opinion the implementation is
> messy.
>
>>>>>> You can't avoid adding the window argument.  Otherwise, you need
>>>>>> to invent such hacks as sending the window selected by the user
>>>>>> to the command body via a symbol property.
>>>>>>
>>>>>> But in the wrapper command like above there is no problem
>>>>>> of adding the window argument to the new command.
>>>>>
>>>>> I wasn't familiar with interactive form enough and didn't know how to connect
>>>>> the optional argument from within interactive form. Symbol-plist was just
>>>>> workaround for lack of a better knowledge :). I stated that explicitly in the
>>>>> mail with the patch, but you perhaps didn't have time to read the mail?
>>>>
>>>> I read all your mails carefully, and noted that there is no better alternative
>>>> than adding the window argument.
>>>
>>> Ok, I am sorry if I missunderstand you, I thought you mean I want add the window
>>> argument to *all commands*, which I don't want. Only those that are called
>>> possibly more then once, either because they are called from other command, or
>>> recursively. The argument is there just to avoid the prompting more then
>>> once. Now if you want to wrap everything, then I think you can avoid the
>>> argument completely *iff* you can switch to the info window before you call the
>>> wrapped command.
>>
>> I agree that the window argument is not needed when you can wrap with e.g.
>>
>>   (with-selected-window (info-window)
>>     (Info-index topic)))
>
> No, not really, window argument is not about wrapping but about prompting, look
> through the code of the patch and you will understand.
>
> It is not needed when there is no risk of prompting user more than once. Again,
> ;), it is needed only in commands that call themselves recursively or in
> commands that are called from other commands, explicitly or via some other
> call. That is actually relatively *small* number of commands. Info-mode does that
> in several places. Help-mode does not do that at all. If you are going to switch
> to info window before you call original command you shouldn't need it at all. If
> you don't prompt user, then you wouldn't need it at all, you would just possibly
> select the same window multiple times, but without visible effect to the user,
> it does not matter.

Please note that risk of prompting user more than once doesn't exist in case
of using wrapper commands.  Only the wrapper command prompts the user.
Then all other intermediate functions called recursively or via
function call chain don't need to prompt the user again because
they all work in the right window.

>>>>> Anyway, I understand now how is it done:
>>>>>
>>>>>       (append (eval (cadr (interactive-form 'Info-index)))
>>>>>               (list (selected-window)))
>>>>>
>>>>> I have to return a list of all arguments from interactive form; it was that
>>>>> simple. Thanks for the example.
>>>>
>>>> Indeed, this is only an example.  More handling may be needed for
>>>> the return value of interactive-form.
>>>
>>> I never wrote complex interactive forms before, so I was a unfamiliar with the
>>> detials and a bit lazy to look around andd see how it is done, so I did a
>>> hack, but from your example I understand now how it is used. It was a couple of
>>> minutes to rework the patch without symbol-plist hack :). Thanks for showing it.
>>>
>>>>>> Maybe it's possible even to write a macro that will generate
>>>>>> such wrapper commands automatically from existing commands.
>>>>>>
>>>>>> It seems you assume that all commands should take a window.
>>>>>
>>>>> Why would I assume that? I wrote explicitly which commands were extended with
>>>>> an optional window argument and why. I don't feel for repeating entire text here,
>>>>> if you have interest, please take a look at the original email, you don't seem
>>>>> to have read it all. That mail answered your opening question, but I didn't want
>>>>> to point it out earlier to not sound impolite, instead I tried to clarify the
>>>>> problems and choices further.
>>>>
>>>> I was referring to your words from another mail that I read carefully:
>>>>
>>>>> What I am trying to say is that command has to be written with the assumptions
>>>>> that user can call it from any window and Emacs frame, and that all prompting
>>>>> should be done in the frame at which user types, so I have reworked info commands
>>>>> to work with those assumptions.
>>>>
>>>> I don't agree that all commands should be written with the assumption
>>>> that they can be called from any window.  Only commands with global
>>>> keybindings usually prepare the buffer with the predefined name and mode,
>>>> then mode-local keybindings and mode-local commands assume that they are
>>>> operating in the right buffer.
>>>
>>> With "all commands" I ment all commands that wish to be callable from other
>>> buffers then those they act on. I didn't mean literally each and every in
>>> existence, since as I mentioned, command in Emacs can be anything, I have one
>>> that shutts down the computer.
>>>
>>> I am not sure if I would draw such absolut divider on what should and what would
>>> should not be written as any-window-command or how to call them. As I wrote I
>>> think it is really up to a command, but yes I do agree that not *all* commands
>>> should be callable from everywhere. I don't think I have enough understanding to
>>> yet draw some hard divider what should be and what should not be callable from
>>> any buffer. What I have got from this experiment is the understanding what is
>>> needed to write such command and what are possible problems. At least I think
>>> so, but there is probably more to it, that I haven't yet encountered.
>>>
>>> What I also think is that you could encapsulate that into a macro, similar to
>>> define-minor-mode, which encapsulate boring details and window switching, so
>>> people don't have to thinkg about it. That would eliminate need for a wrapper
>>> and perhaps made usage of prefix argument more uniform and predicatble. But that
>>> is just an idea, I haven't experimented.
>>
>> Do you mean such a macro will eliminate the need of altering existing commands?
>> This would be nice.  Something like this:
>>
>>   (keymap-global-set "M-i d" (with-window-wrapper 'Info-directory))
>>
>>>>>> But there are no such assumption for most commands that work
>>>>>> only in the selected window.
>>>>>
>>>>> I am not sure what you are trying to say here. If we have several live buffers
>>>>> and wish to act on one, then the user has to choose one somehow, no? We can
>>>>> either try to automate stuff as I have tried in this patch, by prompting the
>>>>> user in ambigous case when system can't figure it on its own, or we can have a 
>>>>> dumb system that forces user to *always* select a window prior to acting on a
>>>>> buffer. If you take a look at the original mail you will understand which commands
>>>>> has got the optional window argument and why, but if you don't prompt the user,
>>>>> than you don't need that argument at all.
>>>>
>>>> Only commands with global keybindings need a way to select the right buffer.
>>>> The best example of such command is the entry point to the Info browser
>>>> the command 'info' bound globally to 'C-h i'.  This command allows the user
>>>> to select the right buffer "*info*<N>" by using the prefix arg.
>>>>
>>>> What you are trying to do is to copy this logic to all Info-local commands.
>>>
>>> I am not sure how you mean to use this, and why do you think C-h i is enough?
>>>
>>> I don't know what your assumption and expectation is. From very beginning I said
>>> I wish to minimize switching between windows. So I do wish more or less all info
>>> and help mode commands to be callable from other windows.
>>>
>>> Also I definitely want to have Info-mode-map on a prefix key, so which-key can
>>> show me the popup, since I have a memory like a gold fish and forgott which key
>>> is bound to what. That was one of reasons why I went away from pre/post hook
>>> thing. To be honest I am not sure I understand how you mean to use the thing.
>>
>> There is no problem with a prefix key since you can create a new keymap
>> with other-window commands like
>
> For me it is because I would prefer having less code if I don't have to. I
> assumed you will do something like that; and my personal opinion is that I would
> definitely prefer not having foo, and foo-other-window, along with foo-mode-map
> and foo-mode-other-window-map, because I see it as a necessary duplication in an
> environment already full with numerous commands and symbols.

It's possible to bind wrappers as lambdas without creating command names,
but this has own disadvantages.

>>   (keymap-global-set "M-i" Info-other-window-map)
>>   (keymap-set Info-other-window-map "d" 'Info-directory-other-window)
>>   ...
>>>> And here comes the disagreement about the ways of doing this.
>>>
>>> I am not sure what I can say here. I understand it is a lot, 2k sloc patch is
>>> big; and I do understand there are potentiallys (or sure) bugs, but the work is
>>> done, I am using it myself, and it seem to be fine. If more people tested it, I
>>> don't think it would be too long before bugs are fixed. In my opinion, if we do
>>> things, lets do them properly from the beginning :). Piling hack on hack will
>>> just lead to more hacks. I don't that is how I see it, but I also don't think it
>>> is very complicated patch conceptually.
>>>
>>> I have attached a patch I reworked today so I can use it with the help-mode and
>>> with removed plist hack. If you look at the help-mode functions, you will see
>>> the change to them is quite trivial. I do disslike the help-mode/info-mode though.
>>>
>>>> I know that you already said about this, so hereby I confirm that I already
>>>> have seen your opinion:t
>>>>
>>>>> Making a wrapper just to put a call to a command into with-selected-window,
>>>>> instead of wrapping the body of that command with the same macro, is not so much
>>>>> better; just different. Sure "less intrusive" on a  command itself, but there is
>>>>> a new command and then we have two where one can do the jobb.
>>>>
>>>> Replying to your opinion, I expressed a preference to avoid massive changes
>>>> in the existing functions.
>>>
>>> I totally understand your sentiment, and I myself would be all for a "magic
>>> pedal" that just switches the right thing on, but there is a limit to hackiness
>>> too.
>>>
>>>>> There is also a problem of prompting and input focus. As I wrote in response to
>>>>> Eli, each command is its own little program, and can prompt user for whatever
>>>>> reason whenever. Thus each command should be written with the assumption that
>>>>> input should be presented to user on the frame where user types and with
>>>>> assumption that user is not executing it from the buffer/window on which it
>>>>> should act. You can achieve all this with tools already in Emacs, no need to
>>>>> introduce any new concepts or macros, and it will also work regardless of the
>>>>> window manager (I think).
>>>>
>>>> I have no idea how this would work for window managers that don't switch focus
>>>> to the frame with the active minibuffer.
>>>
>>> If you pre-select window (and frame) before you call an Info-mode command, than
>>> the focus will be on that frame. You don't need to do anything special, since
>>> with-selected-frame should select frame, window and the visible buffer, so
>>> original command will execute in the "right" context. Problem as stated was,
>>> that when mouse cursor is on the "old frame", the old frame gets focus, which
>>> messes things up. Try to set mouse-autoselect-window in your Emacs, and foucs
>>> follow mouse in window manager, if you use some floating wm. I guess tiling WMs
>>> don't have this problem, but I don't know really, I haven't used anything other
>>> then dwm, and it was long time ago.
>>
>> 'mouse-autoselect-window' has no effect with my WM.  So focus switches to
>> another frame only with 'C-x 5 o'.  But this is not surprising since
>> 'other-frame' at the end explicitly calls 'select-frame-set-input-focus'.
>> There is no such call in 'with-selected-frame'.
>
> I don't think you should set-input-focus for user. The point of the patch is
> that I don't want to switch from the buffer I am working in. If I wanted to
> switch focus and change buffer, I would just code a very simple "jump" function
> as I did initially with jump-info and jump-window.

Yes, I already noticed that you use with-current-buffer instead of with-selected-frame
and then read arguments in the selected frame.

>>>>> It is just that the old commands are not written with those assumptions, so I
>>>>> rewrote Info commands in this patch. I am not sure you can achive that
>>>>> with wrapping, but perhaps there is a way, you can try.
>>>>
>>>> Do you see some needs that can't be achieved with wrapping commands?
>>>
>>> Depends on your ambitions :). Focus and input are potential problems. How do you
>>> plan to use wrappers? If you create double commands, then you will have a bunch of
>>> "-other-window" commands, would you have a separate mode maps for them, or how
>>> do you plan to present them to a user? Which one do you leave out and why? I
>>> personally don't think it is pretty to switch focus to other frame, either in
>>> vertical, left/right setup, or in horizontal tob/bottom setup. I prefer
>>> minibufer popping up on the frame where I already type.
>>
>> Interesting.  So the interactive form doesn't need to switch to another
>> frame with 'with-selected-frame' and can use only 'with-current-buffer',
>> so the minibuffer will use the contents of Info buffer.
>
> You said you have read my mail carefully, but you are constantly asking me
> things that tells me you have neither done that, nor did you bother to look
> through the code or test the patch. Seems like you have just seen a big patch
> and decided you will do it differently? That is a bit sad.

I have read all your mails and whole patch carefully (but can't test it
because your patch does not apply to master/emacs-29).  Now it's difficult
to read this mail because quotes are no more highlighted in Gnus.
I don't know if this is because you mail is too long, or there is
some bug in Gnus, but this is a minor problem.

> The minibuffer does use nothing. If it is enough with just switching to the
> buffer or you need to switch to the window depends on the work in interactive form.
>
> Obviously some functions need access to the content of info-buffer. For example
> Info-menu need acces to the text of an info buffer to create a menu that will be
> presented to the user. Obviously in that case it is enough to just switch to the
> buffer, and since you are just using the buffer you don't need to switch to
> other frame so input is left on the user frame. You can use with-selected-window
> instead, if you take care of focus and input.

Yes, this would be better instead of with-selected-frame.

>>>>> In my opinion wrapping is OK if we for some reason can't alter the code, but in
>>>>> the case of Info and help mode, I don't see such reason, especially since it is
>>>>> possible to do everything backwards compatible. On the negative side,
>>>>> wrapping introduces double set of commands, so what are you saving? You are
>>>>> wrapping code from "outside", while I have done it from "inside", but overall,
>>>>> the principle is the same. On a plus side for wrappers is that you can actually
>>>>> write a codegen for wrappers, hopefully in form of a macro and it will work for
>>>>> other modes. So it is not only negatives, but you could also have both
>>>>> approaches at the same time too :).
>>>>
>>>> Indeed, duplication of the command set is a drawback of wrapping commands.
>>>> OTOH, wrapping commands could be created mechanically, and they are very small.
>>>
>>> I think wrapping the function body in with-selected-window is even smaller, look
>>> at help-mode in attached patch.
>>
>> I don't see how they are smaller.  They are exactly like wrapper commands ;)
>
> They do not introduce an extra symbol, an extra function call and an extra
> documentation string, stashed in an extra keymap, as wrappers do.

I see an extra map as an advantage because users will be able
to put both help-other-window and info-other-window commands
on the same prefix map.  Then e.g. 'M-i l' will go back either
in the nearest Help or Info window.

> The patch is massive because diff has shifted a lot of code; perhaps if I
> stashed my functions on the end of info.el, the change would be less massive? :)

Yes, probably a short section of autoloaded wrapper commands at the end of info.el
would look nicer.

> I don't know, I understand your motiv, but I don't really think that is the best
> way to do things, but seems like I am the only one :).

This is just my opinion.  If people would think your patch is a good way
to go, that would be fine with me.