Closed tuhdo closed 6 years ago
Tu Do notifications@github.com writes:
It would be nice if Helm allows to open its buffer in another frame and quit gracefully.
The problem is to handle persistent actions from another frame without loosing focus to this frame (there is nothing in emacs or in any windowing systems that allow this AFAIK).
The reason is that you can then open a completion frame right below the point (cursor).
Perhaps this could be implemented with limited functionality like no persistent actions.
-- Thierry
@thierryvolpiatto Probably that's good enough for many commands, especially those that uses completion-at-point
.
There's a packaged called framey that already allows to open Helm in another buffer. I haven't tested persistent actions though. You will need to install a few dependencies before you can use it. This is the demo from the author.
Also, this reddit thread.
I forgot to mention that framey is alpha quality. But it’s a propf of concept.
About the focus problem, I think people who embrace frames are aware of it, as they might use a frame for listing grep results. Woth frame, helm-resume should work even better because you only need to switch to a Helm frame to resume a command again.
Tu Do notifications@github.com writes:
@thierryvolpiatto Probably that's good enough for many commands, especially those that uses completion-at-point.
There's a packaged called framey that already allows to open Helm in another buffer. I haven't tested persistent actions though. You will need to install a few dependencies before you can use it. This is the demo from the author.
Also, this reddit thread.
Thanks, I will look into all this soon.
-- Thierry
A basic implementation is done in PR #1938.
Persistent actions splitting helm-window are disabled when using this, also the mini frame displaying helm-buffer is diplayed on top right, which is not the best place probably, also the frame configuration is hardcoded for now.
Merged in master.
You can have now own frame by setting helm-show-completion-display-function
, helm-completion-in-region-display-function
and/or helm-display-function
to helm-display-buffer-in-own-frame
function.
Note that you will not be able to use persistent actions splitting helm window with such configurations.
Great! Thanks.
Is it possible to select which command to use windows? Like, I set the default to use frame, but for some simple commands, it is ok to use a window.
For example, commands like helm-mini
, helm-find-files
, helm-recentf
, etc... I still use windows, but for commands like helm-do-grep
, helm-do-ag
, helm-occur
... are perfect for using frames, as I can resume easily with other-frame
, or simply pressing Alt+Tab.
I think persistent action is still possible with frame, because you already have a Helm frame object. To return to the original frame, you can create a post-persistent-action-hook just to execute a function that returns back.
Also, currently, every time I run a command, a new frame is made and as a result, there is a brief delay. Instead, a frame should be created beforehand, and Helm will always use this frame to display buffer. If, for some reason, it is killed accidentally, then Helm recreates it.
To make the frame cleaner and look like a popup, you can add the attribute (undecorated . t)
. It is cleaner and is also faster to display.
The new frame should stay right below point. This is easy to do. Here is my code that produces a peek frame, you can look at step 1:
(defun make-peek-frame (find-definition-function &rest args)
"Make a new frame for peeking definition"
(when (or (not (rtags-called-interactively-p)) (rtags-sandbox-id-matches))
(let (summary
doc-frame
x y
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 1. Find the absolute position of the current beginning of the symbol at point, ;;
;; in pixels. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(abs-pixel-pos (save-excursion
(beginning-of-thing 'symbol)
(window-absolute-pixel-position))))
(setq x (car abs-pixel-pos))
;; (setq y (cdr abs-pixel-pos))
(setq y (+ (cdr abs-pixel-pos) (frame-char-height)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 2. Create a new invisible frame, with the current buffer in it. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(setq doc-frame (make-frame '((minibuffer . nil)
(name . "*RTags Peek*")
(width . 80)
(visibility . nil)
(height . 15))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 3. Position the new frame right under the beginning of the symbol at point. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(set-frame-position doc-frame x y)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 4. Jump to the symbol at point. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(with-selected-frame doc-frame
(apply find-definition-function args)
(read-only-mode)
(when semantic-stickyfunc-mode (semantic-stickyfunc-mode -1))
(recenter-top-bottom 0))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 5. Make frame visible again ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(make-frame-visible doc-frame))))
Tu Do notifications@github.com writes:
The new frame should stay right below point.
Not necessary, and you don't always have a symbol at point. Try hitting several times C-l and you will see that you don't want necessary to have the frame under point. Having the frame in the middle fix all cases i.e. point at bof, point at eob, and point in the middle of buffer.
-- Thierry
Tu Do notifications@github.com writes:
To make the frame cleaner and look like a popup, you can add the attribute (undecorated . t). It is cleaner and is also faster to display.
Thanks, will look at this.
-- Thierry
Tu Do notifications@github.com writes:
Also, currently, every time I run a command, a new frame is made and as a result, there is a brief delay. Instead, a frame should be created beforehand, and Helm will always use this frame to display buffer.
No, I disagree with this, deleting the frame after each helm session is much cleaner and avoid complications.
-- Thierry
Tu Do notifications@github.com writes:
I think persistent action is still possible with frame, because you already have a Helm frame object. To return to the original frame, you can create a post-persistent-action-hook just to execute a function that returns back.
It is not so simple, so persistent actions splitting helm window are disabled for now until I find a solution.
-- Thierry
Tu Do notifications@github.com writes:
Great! Thanks.
Is it possible to select which command to use windows?
Not necessary commands, but I will try in future to make something more configurable than what we have now.
-- Thierry
No, I disagree with this, deleting the frame after each helm session is much cleaner and avoid complications.
I mean, Helm should only use a single frame to display. So, instead of creating a frame when a Helm command runs, you create it once and only once, then reset it to the default properties. So, every time the frame is shown, it is always the same.
It is not so simple, so persistent actions splitting helm window are disabled for now until I find a solution.
Maybe I should be clearer. My idea is simple like this:
There should be no split window involve. Because in this case, you use a Helm frame purely as a popup. As a popup, window splitting is unnecessary and not desired.
Tu Do notifications@github.com writes:
For example, commands like helm-mini, helm-find-files, helm-recentf, etc... I still use windows, but for commands like helm-do-grep, helm-do-ag, helm-occur... are perfect for using frames,
All these commands are all using persistent actions splitting window...
as I can resume easily with other-frame,
All these commands are already easy to resume AFAIK.
or simply pressing Alt+Tab.
What is Alt+Tab ?
-- Thierry
All these commands are all using persistent actions splitting window...
Can't it be changed to something like e,g, persistent-frame-action
when frame mode is active? For reference, framey is able to split swindow inside its Helm frame. If you want to try, you must install the following packages via MELPA:
shackle
ht
s
dash
Then, load the file framey.el
and activate shackle-mode
. Now, whenever you run a Helm command, it opens in a frame and you can execute persistent action in it without losing focus.
All these commands are already easy to resume AFAIK.
I mean, can I currently resume easily by simply running other-frame
to switch to a Helm frame. I don't even need to run a resume command.
What is Alt+Tab ?
It's a shortcut to switch between programs in non-tiling window manager. I can switch between Helm frame and my main Emacs frame with Alt-Tab
easily.
If you want to return back to the original frame, when creating a new fram, you can set the parameter parent
to contain the current frame, e.g. (parent . (selected-frame))
. Then, when running a persistent action, instead of splitting window, you can retrieve the parent frame with the function frame-parent
to retrieve the original frame, then execute the action in that frame.
It should be doable, unless I'm missing something.
@thierryvolpiatto This is my current code used for creating a frame for xref-find-definitions
and others. You can see that there is always one frame exists - peek-frame-main
- to be called for creating the very first peek frame. peek-frame-main
is created once and only once, then later calls, it will be reused. If there are more frames to be created, they are put in peek-frame-list
and are deleted using the command tuhdo/delete-peek-frames
. All peek frames are deleted, except for peek-frame-main
.
In the case of Helm, it should be easier since you don't need to keep more than 1 frame.
(defvar peek-frame-main nil)
(defvar peek-frame-list nil)
(defun tuhdo/delete-peek-frames ()
(interactive)
(while peek-frame-list
(delete-frame (pop peek-frame-list)))
(when (frame-visible-p peek-frame-main)
(make-frame-invisible peek-frame-main)
(select-frame (frame-parent peek-frame-main))))
(defun tuhdo/xref-peek-definition ()
(interactive)
(make-peek-frame (lambda () (xref-find-definitions (thing-at-point 'symbol)))))
(defun make-default-peek-frame (&optional p)
(make-frame `(
(minibuffer . nil)
(parent-frame . p)
(name . "*RTags Peek*")
(border-width . 0)
(internal-border-width . 0)
(no-special-glyphs . t)
(menu-bar-lines . 0)
(tool-bar-lines . 0)
(left-fringe . 0)
(right-fringe . 0)
(tool-bar-lines . 0)
(line-spacing . 0)
(background-color . "light yellow")
(no-other-frame . t)
(unsplittable . t)
(vertical-scroll-bars . nil)
;; (mode-line-format . nil)
(horizontal-scroll-bars . nil)
(undecorated . t)
(visibility . nil)
(width . 80)
(height . 20))))
(defun make-peek-frame (find-definition-function &rest args)
"Make a new frame for peeking definition"
(when (or (not (rtags-called-interactively-p)) (rtags-sandbox-id-matches))
(let (summary
doc-frame
x y diff
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 1. Find the absolute position of the current beginning of the symbol at point, ;;
;; in pixels. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(line-pixel-pos (save-excursion
(beginning-of-line)
(window-absolute-pixel-position)))
(symbol-pixel-pos (save-excursion
(beginning-of-thing 'symbol)
(window-absolute-pixel-position))))
(setq diff (- (car symbol-pixel-pos) (car line-pixel-pos)))
(setq x (- (car symbol-pixel-pos) (if (< diff 80)
diff
80)))
(setq y (+ (cdr symbol-pixel-pos) (frame-char-height)))
(unless (frame-live-p peek-frame-main)
(setq peek-frame-main (make-default-peek-frame (selected-frame))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 2. Create a new invisible frame, with the current buffer in it. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(when (frame-visible-p peek-frame-main)
(setq doc-frame (make-default-peek-frame))
(push doc-frame peek-frame-list))
(unless doc-frame
(setq doc-frame peek-frame-main))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 3. Position the new frame right under the beginning of the symbol at point. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(set-frame-position doc-frame x y)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 4. Jump to the symbol at point. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(select-frame doc-frame)
(apply find-definition-function args)
(when semantic-stickyfunc-mode (semantic-stickyfunc-mode -1))
(recenter-top-bottom 0)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 5. Make frame visible again ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(make-frame-visible doc-frame))))
Now PA are working.
I've tried it and that's really awesome for PA from other frame! Some more improvements:
Helm Frame should appear under the current point. I have an ultra wide monitor (only 29 inch), but whenever Helm frame appears, it stays on the far left if my Emacs is on the right side of the monitor.
If I set this window configuration, Helm frame won't appear:
(add-to-list 'display-buffer-alist
`(,(rx bos "*helm" (* not-newline) "*" eos)
(display-buffer-in-side-window)
(inhibit-same-window . t)
(window-height . 0.22)))
Also, not sure you’ve done this, but it might speed up frame displaying. In my code, you can see that I only make a frame visible once everything is done, e.g. jumping to definition and recenter. It make the frame appear with content instantly. Currently, Helm frame appears with a blank buffer for a brief moment before displaying its buffer.
Tu Do notifications@github.com writes:
I've tried it and that's really awesome for PA from other frame! Some more improvements:
- Helm Frame should appear under the current point.
As I said not necessarily, if point is at bottom of buffer and your emacs frame is at the bottom of screen, you want your helm frame above point.
I have an ultra wide monitor (only 29 inch), but whenever Helm frame appears, it stays on the far left if my Emacs is on the right side of the monitor.
Hmm, I can reproduce this on my laptop with a small emacs frame on the right side of monitor, don't know for now how to fix this i.e. setup left and top frame parameters.
If I set this window configuration, Helm frame won't appear:
(add-to-list 'display-buffer-alist `(,(rx bos "helm" ( not-newline) "*" eos) (display-buffer-in-side-window) (inhibit-same-window . t) (window-height . 0.22)))
Ok fixed, however are you using this to override helm-default-display-function ? (Because I have disabled also the possibility to do this in this function).
- I should be able to set undecorated frame and the minibuffer for a minimal popup. I think the frame is solely for Helm, it should be minimal.
undecorated
is not available in 25.3, it have no effect here, same as
parent
and maybe others (frame parameters are badly documented or I
missed them).
For minibuffer we need a minibuffer in the helm frame, otherwise user is typing text blindly, which is no good.
-- Thierry
Typing on my phone so unable to quote.
About the frame position ossue with large monitor, that’s why I suggest to put frame under point (it’s done in my code). You can position the frame under point, but frame slightly moved to the left by an offset, like 10 characters.
About the display buffer alist, I use it to always open Helm at the bottom.
About the minibuffer, I actually use Helm as with the feature implemented in this issue: https://github.com/emacs-helm/helm/issues/1064 . A minibuffer is not needed if you use Helm like that.
Undecorated and parent are on Emacs 26. Maybe you can enable with Emacs 26 and test.
Tu Do notifications@github.com writes:
Typing on my phone so unable to quote.
About the frame position ossue with large monitor, that’s why I suggest to put frame under point (it’s done in my code). You can position the frame under point, but frame slightly moved to the left by an offset, like 10 characters.
I have improved the situation, I hope, not perfect though but should be better.
About the display buffer alist, I use it to always open Helm at the bottom.
Well helm provide easier way to do this that hacking display-buffer-alist. I haven't restricted this possibility though.
About the minibuffer, I actually use Helm as with the feature implemented in this issue: #1064 . A minibuffer is not needed if you use Helm like that.
Ok no minibuffer provided in this case, up to you to hide minibuffer in main frame though.
(add-hook 'helm-minibuffer-set-up-hook 'helm-hide-minibuffer-maybe)
-- Thierry
Here is my improved version with the following enhancements:
Using a single frame to speed up displaying of Helm buffer. Without this feature, using Helm with frame is ery slow.
Improved frame position: now, when a new frame appears, by default, the frame is positioned such that its point after pattern:
prompt appears right above/below the point of the original main frame.
Here is the patch:
+++ helm-improved-frame.el 2018-01-03 03:17:55.825597660 +0700
@@ -2598,6 +2598,18 @@ value of `helm-full-frame' or `helm-spli
(window-width . ,helm-display-buffer-default-width))))
(helm-log-run-hook 'helm-window-configuration-hook)))
+(defvar helm-persistent-frame nil)
+(defun helm-display-buffer-popup-frame (buffer frame-alist)
+ (unless (frame-live-p helm-persistent-frame)
+ (setq helm-persistent-frame (make-frame frame-alist)))
+ (select-frame helm-persistent-frame)
+ (let ((x (cdr (assoc 'left frame-alist)))
+ (y (cdr (assoc 'top frame-alist))))
+ (set-frame-position helm-persistent-frame x y))
+ (switch-to-buffer buffer)
+ (make-frame-visible helm-persistent-frame)
+ )
+
(defun helm-display-buffer-in-own-frame (buffer)
"Display `helm-buffer' in a separate frame.
@@ -2617,7 +2629,11 @@ configure frame size."
`((width . ,helm-display-buffer-width)
(height . ,helm-display-buffer-height)
(tool-bar-lines . 0)
- (left . ,(car pos))
+ (visible . nil)
+ (left . ,(- (car pos) (* (frame-char-width)
+ (if (< (- (point) (point-at-bol)) 9)
+ (- (point) (point-at-bol))
+ 9))))
;; Try to put frame at the best possible place.
;; Frame should be below point if enough
;; place, otherwise above point and
@@ -2625,18 +2641,24 @@ configure frame size."
;; by helm frame.
(top . ,(if (> (cdr pos) half-screen-size)
;; Above point
- (- (cdr pos) half-screen-size)
+ (- (cdr pos) (* (frame-char-height) (- helm-display-buffer-height 4)))
;; Below point
(+ (cdr pos) (frame-char-height))))
(title . "Helm")
(vertical-scroll-bars . nil)
+ ,(when (version<= "26" emacs-version)
+ '(undecorated . t))
(menu-bar-lines . 0)
(fullscreen . nil)
- (minibuffer . ,(null helm-echo-input-in-header-line))))
+ ;; (minibuffer . ,(or (null helm-echo-input-in-header-line)
+ ;; (> (cdr pos) half-screen-size)))
+ ))
display-buffer-alist)
- (display-buffer
- buffer '(display-buffer-pop-up-frame . nil)))
- (helm-log-run-hook 'helm-window-configuration-hook)))
+ (if (> (cdr pos) half-screen-size)
+ (setq helm-echo-input-in-header-line nil)
+ (setq helm-echo-input-in-header-line t))
+ (helm-display-buffer-popup-frame buffer default-frame-alist)
+ (helm-log-run-hook 'helm-window-configuration-hook))))
;;; Initialize
@@ -3050,7 +3072,8 @@ WARNING: Do not use this mode yourself,
;; Be sure we call this from helm-buffer.
(helm-funcall-foreach 'cleanup)
(when helm--buffer-in-new-frame-p
- (delete-frame)))
+ (make-frame-invisible)
+ ))
(helm-kill-async-processes)
;; Remove the temporary hooks added
;; by `with-helm-temp-hook' that
Demo:
Tu Do notifications@github.com writes:
Here is my improved version with the following enhancements:
- Using a single frame to speed up displaying of Helm buffer. Without this feature, using Helm with frame is ery slow.
You exagerate! it is not "very" slow, it is really usable. Thus keeping and existing frame will raise that frame sooner or later, which is annoying (I personally use other-window with ALL-FRAMES parameter).
- Improved frame position: now, when a new frame appears, by default, the frame is positioned such that its point after pattern: prompt appears right above/below the point of the original main frame.
The frame position is maybe good on your screen but not here, it is hiding current line as soon as this line reach more than half of screen.
Using alternatively header-line and minibuffer is a good idea.
Here is the patch:
Patch on what? Where?
I had to manually merge the code from your patch to helm.el.
-- Thierry
You exagerate! it is not "very" slow, it is really usable. Thus keeping and existing frame will raise that frame sooner or later, which is annoying (I personally use other-window with ALL-FRAMES parameter).
The frame is invisible, you cannot raise the frame and whenever Helm is done, it is hidden (invisible). Visible/invisible is different than raise/lower frame. When a frame is invisible, even the system window manager cannot see it. You have no way to switch to an invisible frame, except from Emacs. And creating a fresh frame is really much slower. You can try using Helm with frame for an hour, and given how frequently you use Helm commands, it becomes very annoying for 1 second delay.
The frame position is maybe good on your screen but not here, it is hiding current line as soon as this line reach more than half of screen.
So, it is correct for the upper half? Probably due to font height and line spacing. You can tweak to your liking, but ideally it should look the same as in the demo screenshot.
Patch on what? Where? I had to manually merge the code from your patch to helm.el.
I used diff -pu
, but my file name is different. Sorry, I will test the patch next time. I just want you to see the quick changes.
You can read more on Visibility of Frames here: https://www.gnu.org/software/emacs/manual/html_node/elisp/Visibility-of-Frames.html to see that visible/invisible of a frame is different from raise/lower frame.
Tu Do notifications@github.com writes:
You can read more on Visibility of Frames here: https://www.gnu.org/software/emacs/manual/html_node/elisp/Visibility-of-Frames.html to see that visible/invisible of a frame is different from raise/lower frame.
I know all this, this is not the point.
-- Thierry
I know all this, this is not the point.
Well then, what is wrong with an invisible frame?
Tu Do notifications@github.com writes:
I know all this, this is not the point.
Well then, what is wrong with an invisible frame?
1) Use helm from a full screen emacs the frame popup. 2) C-g ok the helm frame is now invisible. 3) Use (other-window 1 t) the helm frame is now visible. 4) Use (other-window 1 t) you are now in main frame and the helm frame is no more invisible, (with a transparent emacs you can see it). 5) Use helm again, the frame is staying behind the main frame (focus stays in main frame). 6) Use (other-window 1 t) again to gain focus in helm frame.
-- Thierry
@thierryvolpiatto Invisible frame does not work like that. Once it is invisible, it won't be visible until you call make-frame-visible
. You can try this:
(make-frame '((visibility . nil)))
(other-window 1 t)
Not sure about Emacs 25.3, if frame visibility behaves differently.
Tu Do notifications@github.com writes:
Once it is invisible, it won't be visible until you call make-frame-visible. You can try this:
- Run this elisp code:
(make-frame '((visibility . nil)))
- Now, try switching window with your code:
(other-window 1 t)
Sorry yes, use this instead:
(defun other-window-forward (&optional n) (interactive "p") (other-window n t) (select-frame-set-input-focus (selected-frame)))
Many people use this (I have this in .emacs since years) to switch in buffers located in other frames, specially useful when using dedicated frames.
One of the problem with invisible frame not raising is solved by using raise-frame after making frame visible.
So, I have a patch ready for providing the two possibilities:
1) pop up a new frame at each time and delete it, this is the current behavior and the most convenient, contrarily to what you say, there is no noticable delay, this will be the default.
2) Reusing the frame, with the disadvantage described above and the advantage you described (you win 0.1 second maximum).
This would be controlled by a variable
helm-display-buffer-reuse-frame
.
Another possibility is just providing a cleanup function variable which
default to delete-frame
so that you can use it with your code by
binding it to make-frame-invisible
, this would simplify the code in
helm by just providing the "popup a new frame and delete it" which will
satisfy most people.
So I don't know yet what I will provide.
-- Thierry
@thierryvolpiatto It is very noticeable:
helm-display-buffer-in-own-frame 1 0.315819644 0.315819644
helm-display-buffer-in-own-frame 1 0.04764237 0.04764237
That does not even count the time for rendering the frame. I am just trying to make this feature more responsive.
This would be controlled by a variable
helm-display-buffer-reuse-frame
.
I would choose this approach. The reuse frame should be hidden and every time it is restored, its property is reset similar to what I did in helm-display-buffer-popup-frame
.
Here are the demos between two implementations:
Tu Do notifications@github.com writes:
@thierryvolpiatto It is very noticeable:
- Creating frame by default:
helm-display-buffer-in-own-frame 1 0.315819644 0.315819644
- Reuse the frame:
helm-display-buffer-in-own-frame 1 0.04764237 0.04764237
That does not even count the time for rendering the frame. I am just trying to make this feature more responsive.
This would be controlled by a variable helm-display-buffer-reuse-frame.
I would choose this approach. The reuse frame should be hidden and every time it is restored, its property is reset similar to what I did in helm-display-buffer-popup-frame.
Ok done, now you can just setq helm-display-buffer-reuse-frame (default is nil).
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.*
-- Thierry
Tu Do notifications@github.com writes:
Here are the demos between two implementations:
- By creating a new frame, you clearly see a brief delay with a blank frame:
helm-new
- By reusing a frame, the Helm frame appears almost instantly:
Really this make no difference for me, but if it does for you now you can reuse frame.
Thanks for helping on this.
-- Thierry
Great! Now it's better than doing it manually in my init file. A few suggestions:
We should have an exclude list. The idea is that instead of setting display function, there should be a helm-frame-mode
, a minor mode. Then, when a helm command is ran, Helm checks the exclude list for commands that shouldn't be displayed in a frame, but by a window. When helm-frame-mode
is disabled, which means window is used by default, then the exclude list specifies the commands that should be displayed in a frame.
Frame positioning should be improved for lower half of the screen. Currently, it is offset by 5 lines on my screen. Probably you should look at my old formula in the diff, and adjust it a bit. For example, in the original diff:
(- (cdr pos) (* (frame-char-height) (- helm-display-buffer-height 4)))
It subtracts 4 to the number of lines. You can subtract more, e.g. 5 or 6, it might looks good on your screen, which probably looks good on mine too.
Also, for the left side of the frame, I also did:
(if (< (- (point) (point-at-bol)) 9)
(- (point) (point-at-bol))
9))))
It is meant to make the frame cursor and the current cursor aligned vertically, thus making the transition from current point to frame point smoother.
Tu Do notifications@github.com writes:
Great! Now it's better than doing it manually in my init file. A few suggestions:
- We should have an exclude list. The idea is that instead of setting display function, there should be a helm-frame-mode, a minor mode. Then, when a helm command is ran, Helm checks the exclude list for commands that shouldn't be displayed in a frame, but by a window. When helm-frame-mode is disabled, which means window is used by default, then the exclude list specifies the commands that should be displayed in a frame.
Yes, probably something like this, for now it is easy to set which command use frame by advicing helm commands see helm-display-function docstring.
- Frame positioning should be improved for lower half of the screen. Currently, it is offset by 5 lines on my screen. Probably you should look at my old formula in the diff, and adjust it a bit.
I tried it, it doesn't work for me, the frame is hiding current pos in many places.
For example, in the original diff:
(- (cdr pos) (* (frame-char-height) (- helm-display-buffer-height 4)))
It subtracts 4 to the number of lines. You can subtract more, e.g. 5 or 6, it might looks good on your screen, which probably looks good on mine too.
Also, for the left side of the frame, I also did:
(if (< (- (point) (point-at-bol)) 9) (- (point) (point-at-bol)) 9))))
It is meant to make the frame cursor and the current cursor aligned vertically, thus making the transition from current point to frame point smoother.
I think it is really hard to have something perfect for every ones, what you are using above is not working at all for me for example and I am not surprized that the current default is not working for you, it depends on too many things, screen, window manager etc..
-- Thierry
It is strange that a formula based on character width and height does not work, as it is supposed to be. Probably there are more parameters to consider, like line spacing. I currently have 2 screens, one is my MacBook and the other is my external screen. I will test on both tomorrow, on Windows, Linux and Mac OS.
This could potential be a major reason: I used undecorated attribute when setting those formula. You use Emacs 25.3, the attribute is unavailable so the frame title bar is visible, and it is indeed depends on the current window manager. Maybe if there is a way to retrieve the height of the title bar, it will be accurate.
It seems we can find the height by subtracts frame-textarea-height from frame-height. Worth a try.
Or even better, you can use frame-edges to get the height diẻctly: https://www.gnu.org/software/emacs/manual/html_node/elisp/Frame-Layout.html
Tu Do notifications@github.com writes:
It seems we can find the height by subtracts frame-textarea-height from frame-height. Worth a try.
The trap is trying to calculate according to frame size, you should not take in account the frame size but the screen size and your actual position on screen, not frame or window.
-- Thierry
It would be nice if Helm allows to open its buffer in another frame and quit gracefully. The reason is that you can then open a completion frame right below the point (cursor).