Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide ace-window-posframe-mode #192

Closed
wants to merge 3 commits into from

Conversation

notmgsk
Copy link
Contributor

@notmgsk notmgsk commented Apr 4, 2020

Use posframes for overlays.

There does seem to be a slight but noticable delay when displaying the posframes. Haven't investigated too much into why.

@abo-abo
Copy link
Owner

abo-abo commented Apr 28, 2020

Thanks. Looks good. Do you have an Emacs Copyright Assignment? Both ace-window and posframe are on https://elpa.gnu.org/packages/.

@notmgsk
Copy link
Contributor Author

notmgsk commented Apr 28, 2020

Working on it. Will let you know once it's done.

@abo-abo
Copy link
Owner

abo-abo commented Apr 28, 2020

Thanks in advance.

@obar
Copy link

obar commented Jun 16, 2020

I'm using this with (set-face-attribute 'aw-leading-char-face nil :weight 'bold :height 2.0) and the height scale in particular makes the labels stand out. I think it's probably too opinionated to set as a default, but perhaps it warrants inclusion in an example adjacent to the documentation for ace-window-posframe-mode in the eventual readme.

This is a useful patch for a great package! Thanks to you both.

:poshandler aw-posframe-position-handler
:font (face-font 'aw-leading-char-face)
:foreground-color (face-foreground 'aw-leading-char-face)
:background-color (face-background 'aw-leading-char-face)))))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm excited to have this functionality in ace-window mainly for use with exwm. For that, though, I need to add the (parent-frame . nil) parameter to posframes. So a defcustom aw-posframe-parameters would be lovely.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does (parent-frame . nil) work well?

:background-color (face-background 'aw-leading-char-face)))))

(defun aw--remove-leading-chars-posframe ()
(map nil #'posframe-delete aw--posframe-frames))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my ignorance, but while trying this I got an error about map not being defined. I don't know which package should provide this (I tried to install and load a package called "map", but it didn't work). A simpler

(mapcar #'posframe-delete aw--posframe-frames)

seems to work. (I only did some quick testing).

Another thing is that using your fork, posframe is not listed as dependency (or at least straight.el doesn't think it is -- I am not yet aware of how dependencies are declared and consumed.)

Anyway, thanks for this feature, is awesome :)

@notmgsk
Copy link
Contributor Author

notmgsk commented Jan 23, 2021

@abo-abo Finally I have the copyright assignment. I'm going to take a look at this again, and address the comments above.

@gcv
Copy link

gcv commented Mar 2, 2021

@notmgsk: The posframe display delay probably occurs because posframe creation takes a bit of time. You can mitigate the problem by caching and hiding posframes (and lazily creating them as needed, and/or pre-creating a bunch when Emacs idles).

@notmgsk
Copy link
Contributor Author

notmgsk commented Mar 2, 2021

@notmgsk: The posframe display delay probably occurs because posframe creation takes a bit of time. You can mitigate the problem by caching and hiding posframes (and lazily creating them as needed, and/or pre-creating a bunch when Emacs idles).

Thanks. The lazy approach is already implemented, and has been sufficient for my usage.

;; something like: a frame exists which hasn't been deleted
;; (with posframe-delete) and has the same configuration as
;; the requested new frame.
(bufname (format "*aw-posframe-buffer-%s*" path)))
Copy link

@tumashu tumashu Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe better?

 (bufname (format " *aw-posframe-buffer-%s*" path))) 

@josephmturner
Copy link

Hello! Thank you for ace-window! Is there a reason to not merge this PR?

@abo-abo abo-abo closed this in e3e6ec1 Sep 6, 2022
@abo-abo
Copy link
Owner

abo-abo commented Sep 6, 2022

Merged. Thanks @notmgsk for the contribution. And to everyone who reviewed. And @josephmturner for the ping.

@josephmturner
Copy link

@abo-abo @notmgsk Thank you!! This will make ace-window work much better with EXWM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants