Re: Info-mode patch

[Arthur Miller]

Juri Linkov <juri@linkov.net> writes:

>>>>>>> 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.

>>>>>> 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.

>>>> 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.

>   (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.

>>>> 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.

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.

>>>> 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.

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? :)

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 :).