search

abo-abo / ace-window

Quickly switch windows in Emacs
977 stars 87 forks source link

'z' window identifier creates a new frame as a target; mouse click target selection; aw-mode string in minibuffer #114

Closed rswgnu closed 6 years ago

rswgnu commented 6 years ago

This pull request makes 4 changes across 3 commits.

Commit 1: This allows specifying a new frame as the window target. A special character defined by aw-make-frame-char (default = z) means create a new frame and use its window as the target. The new frame's location is set relative to the prior selected frame's location and given by aw-frame-offset. The new frame's size is given by aw-frame-size. See their documentation strings for more information. This is an implementation of Issue #110.

Commit 2: This removes the 'i' command binding (duplicate of 'o') for Hyperbole compatibility and updates the README.md to the latest set of commands.

Commit 3: Adds mouse selection of the target window. Although ace-window is meant to speed keyboard selection of windows, sometimes it is useful to combine the keyboard with the mouse. This change allows use of the keyboard or mouse key clicks any time ace-window prompts for a window.

The same commit also adds aw-minibuffer-flag which when non-nil also displays the `ace-window-mode' string in the minibuffer since it is often not fully visible in the modeline when there are many windows in a frame. It is off by default to maintain backward compatibility.

abo-abo commented 6 years ago

What's the reason for introducing aw-avy-read?

rswgnu commented 6 years ago

On Mon, Nov 27, 2017 at 1:15 PM, Oleh Krehel notifications@github.com wrote:

What's the reason for introducing aw-avy-read?

​avy-read exits before storing the full path of a tree leaf into avy-current-path which is used to determine when the path matches 'z' and therefore should be used to create a new frame (e.g. wouldn't want to match to the 'lz' path). aw-avy-read fixes this. I meant to mention it to you. You may just want to change avy-read to do the same but I did not know for sure if this would affect any of its mechanics and I did not want the pull request to have to change​ anything in avy.

rswgnu commented 6 years ago

How does this PR look? I'd love to get it integrated soon. Thanks.

abo-abo commented 6 years ago

That's a lot to review. I'll do it on the weekend. I want to avoid introducing a variant of avy-read.

rswgnu commented 6 years ago

That's a lot to review. I'll do it on the weekend. I want to avoid introducing a variant of avy-read.

Sure.

I think you can make the same change to avy-read directly. I haven't seen any issue but I use ace-window only right now, so I don't know how you might use that value in any other package.

Cheers,

Bob

rswgnu commented 6 years ago

See the updated first comment in pull #114 for a description of changes.

abo-abo commented 6 years ago

What's the use case of mouse selection? If you use the mouse, you don't need ace-window?

aw-avy-read

Please rework the PR to make aw-avy-read re-use the code of avy-read instead of copying it. It looks like it's enough to bind avy-handler-function and call avy-read.

rswgnu commented 6 years ago

On Fri, Dec 8, 2017 at 1:42 PM, Oleh Krehel notifications@github.com wrote:

What's the use case of mouse selection? If you use the mouse, you don't need ace-window?

​1. You want to use ace-window for its commands, like swap-window, but you have your hand on the mouse, so you click it to select the target window using any mouse button. It is possible the mouse might be used via a special location click to invoke ace-window as well so the whole command could be done via the mouse.

  1. With Hyperbole, you might perform a drag action with the mouse that starts an ace-window command, so you select things with the mouse.

aw-avy-read

Please rework the PR to make aw-avy-read re-use the code of avy-read instead of copying it. It looks like it's enough to bind avy-handler-function and call avy-read.

​Ok, I'll look at that.

Bob ​

rswgnu commented 6 years ago

I eliminated the duplication of avy-read and just made the changes necessary to it in avy. I could not see how to make things work properly just by setting the handler function because of certain character processing within avy-read. This has the benefit that packages other than ace-window can utilize the mouse press technique as well.

abo-abo commented 6 years ago

Thanks. Merged with heavy changes. Please review the commits, see if anything is missing. Also, please use less code per PR next time, it's easier to understand each other if there's less code.

Here's the digest of the first commit:

(defun aw-use-frame (window)
  (aw-switch-to-window window)
  (aw-make-frame))

(add-to-list 'aw-dispatch-alist
             '(?z aw-use-frame "Use new frame"))

No API functions needed to be rewritten after all.

rswgnu commented 6 years ago

Thanks for integrating this.

There is something wrong with the your edits though. 'z' should work only as a window target, not as an ace-window command. Say ace-window is bound to M-o. Start a fresh Emacs and type: M-o z That works properly and creates the new frame. Now do it again. M-o z That is still waiting for a window target. I have only one window, labeled 'a'. So I press 'a' and then the new frame is created, as if z were used as a command, not a window target. Just FYI, the original PR did not display this behavior based on prior testing.

rswgnu commented 6 years ago

The reason the PR was so large is that within a single branch, Github seems to build up all commits into a single PR until they are merged, so even though features were committed separately, they ended up as one large PR. I will have to look into how to created multiple PRs at the same time though I think it will require creating different branches.

rswgnu commented 6 years ago

Remove 'z' from the dispatch-alist where the commands are. Process it as a window target only and that should resolve things. Maybe just look at that part of my code again. Of course, you can't add it to the lead characters since no window yet exists in which to display it since it creates the window in the new frame.

abo-abo commented 6 years ago

I've misunderstood some part of the PR. I add a small correction. Please correct further if needed in a new PR.

eflanigan00 commented 6 years ago

Really good work. I would recommend you make 2 open a frame because this is more consistent with the traditional emacs commands. Of course everything is customizable. Some other ideas on the wiki:

https://github.com/abo-abo/ace-window/wiki/Hydra

rswgnu commented 6 years ago

On Sun, Dec 17, 2017 at 10:50 PM, eflanigan00 notifications@github.com wrote:

Really good work. I would recommend you make 2 open a frame because this is more consistent with the traditional emacs commands.

​I personally use a Hydra on C-z for all frame control commands. I find this parallels C-x window commands much better. Additionally, a recommended setting for ace-window label characters is to use the home key letters, so having this be a letter matches up there as well and it is easier to type. ​​​

Of course everything is customiza ​​ ​​ ble.

​Yes, it is a variable so you can set it however you like.​​

Some other ​​ ​​ ​​ ​​ ​​ ​​ ​​ ​​ ​​ ​​ ideas on the wiki: ​​ ​​

htt https://github.com/abo-abo/ace-window/wiki/Hydra ​​ https://github.com/abo-abo/ace-window/wiki/Hydra ps://github.com/abo-abo/ace-wi ​​ ndow/wiki/Hydra https://github.com/abo-abo/ace-window/wiki/Hydra ​​

​I look forward to checking this out.