tumashu / company-posframe

121 stars 24 forks source link

Add configuration option to allow showing the dialog at point #47

Closed yyoncho closed 4 years ago

yyoncho commented 4 years ago

#

tumashu commented 4 years ago

maybe we should use company-posframe-show-params instead of add another custom variable.

yyoncho commented 4 years ago

maybe we should use company-posframe-show-params instead of add another custom variable.

That would work if posframe allows passing functions as arguments, e. g.

(posframe-show 
   ...
   :position #'point
   ...)
tumashu commented 4 years ago

posframe support position-function, :poshandler

yyoncho commented 4 years ago

posframe support position-function, :poshandler

yes, but we will still need to handle :x-pixel-offset

tumashu commented 4 years ago
yes, but we will still need to handle :x-pixel-offset

poshandler return pixel x and y, so you can just - offset in poshandler function.

tumashu commented 4 years ago

by the way, x-pixel-offset and y-pixel-offset has been include into poshandler's argument

      (posframe--set-frame-position
       posframe
       (posframe-run-poshandler
        ;; All poshandlers will get info from this plist.
        (list :position position
              :position-info position-info
              :poshandler poshandler
              :font-height font-height
              :font-width font-width
              :posframe posframe
              :posframe-width (frame-pixel-width posframe)
              :posframe-height (frame-pixel-height posframe)
              :posframe-buffer buffer
              :parent-frame parent-frame
              :parent-frame-width parent-frame-width
              :parent-frame-height parent-frame-height
              :parent-window parent-window
              :parent-window-top parent-window-top
              :parent-window-left parent-window-left
              :parent-window-width parent-window-width
              :parent-window-height parent-window-height
              :mode-line-height mode-line-height
              :minibuffer-height minibuffer-height
              :header-line-height header-line-height
              :tab-line-height tab-line-height
              :x-pixel-offset x-pixel-offset
              :y-pixel-offset y-pixel-offset))
       parent-frame-width parent-frame-height)
yyoncho commented 4 years ago

poshandler return pixel x and y, so you can just - offset in poshandler function.

Ok, I will restructure the PR to use poshandler.

yyoncho commented 4 years ago

@tumashu can you help with implementing the poshandler which will show at company-prefix point?

yyoncho commented 4 years ago

@tumashu can you help with implementing the poshandler which will show at company-prefix point?

@tumashu - I pushed a change which illustrates the structure of the code with company-posframe-show-at-prefix calling posframe-poshandler-point-bottom-left-corner which has to be changed to place the dialog at company-prefix.

yyoncho commented 4 years ago

@tumashu ignore the last two comments, I was able to make it work.

tumashu commented 4 years ago

what about add a new custom variable, for example: posframe-company-poshandler ?

yyoncho commented 4 years ago

what about add a new custom variable, for example: posframe-company-poshandler ?

I am fine with that approach as well but this will kind of duplicate the company-posframe-show-params.

tumashu commented 4 years ago

you need to deal with x offset: (* -1 company-tooltip-margin (default-font-width)) too.

yyoncho commented 4 years ago

you need to deal with x offset: (* -1 company-tooltip-margin (default-font-width)) too.

I am including company-tooltip-margin in position-info calculation, this seems to be sufficient in my testing.

tumashu commented 4 years ago

before company-posframe-show-params is default nil, if we add poshandler to default, some user's config may work unexpect for it edit this variable. so add a new variable may better

yyoncho commented 4 years ago

before company-posframe-show-params is default nil, if we add poshandler to default, some user's config may work unexpect for it edit this variable. so add a new variable may better

Make sense, I will change it to variable.