Conflict between company-tng and company-preview-frontend

[emacs-evil]

Conflict between company-tng and company-preview-frontend

[jojojames]

It might be better to take a close look at that thread.

For reference, I do use those keys too.

(let ((keymap company-active-map))
      (define-key keymap [return] '+company-expand-yasnippet-or-return)
      (define-key keymap (kbd "RET") '+company-expand-yasnippet-or-return)
      (define-key keymap "\C-n" 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap [tab] 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap (kbd "TAB") 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap [S-tab] 'company-select-previous)
      (define-key keymap [S-iso-lefttab] 'company-select-previous)
      (define-key keymap [(shift tab)] 'company-select-previous)
      (define-key keymap [backtab] 'company-select-previous)
      (define-key keymap "\C-p" 'company-select-previous)
      (define-key keymap [backtab] 'company-select-previous)
      (define-key keymap (kbd "S-TAB") 'company-select-previous))

[jojojames]

company-mode/company-mode#706

I use my own modified version so I can use company-preview-frontend.

[jojojames]

Just to clarify: It's not something we can expect upstream to fix.

[Ambrevar]

What's your proposed fix?

Mine is

  (define-key company-active-map (kbd "<tab>") 'company-select-next-if-tooltip-visible-or-complete-selection)

[Ambrevar]

Why would not upstream bind to company-select-next-if-tooltip-visible-or-complete-selection instead?

[Ambrevar]

I did read the thread (and just re-read it) but I'm not sure I get your point. Is it because that would break yasnippet? I don't use snippets and I don't use company much to be honest, so I'm a little ignorant in that area.

On a different topic, why do you double-assign your bindings with vector syntax and kbd syntax (e.g. [return] and (kbd "RET"))?

[Ambrevar]

If the fix cannot go upstream, I don't think it should go here either, so I suggest we just add to evil-company an explanatory comment with workaround.

[jojojames]

Pasting a comment from that thread.

Problem #1

Noticed there's an extra preview added when combining tng with preview frontend.

Problem #2

Also became tricky to expand snippets when there's more than one. Using RET for that seems like a good idea.

On a different topic, why do you double-assign your bindings with vector syntax and kbd syntax (e.g. [return] and (kbd "RET"))?

No good reason.

[Ambrevar]

OK, I think I get it, and company-mode/company-mode#706 (comment) suggests we do our own bindings.
But

(define-key keymap [tab] 'company-select-next-if-tooltip-visible-or-complete-selection)

would work everywhere I believe, and the thread did not mention it. You sure upstream would not consider it?

[jojojames]

Could you clarify what you expect to happen (and what does happen) when you use eshell+company+tng

Referencing this:

With evil-collection, we enable company-tng through company-tng-configure-default which rebinds TAB and thus makes it impossible to confirm suggestions in Eshell.

Just want to make sure we're on the same page.

[Ambrevar]

Yeah, I was starting to think we are talking about two different problems. But I think they overlap.
With Eshell:

$ ls -la
...
$ ls<|>

<|> is the point. If I wait company-idle-delay, then I get a -la preview thanks to company-eshell-autosuggest.
At this point, I should press TAB to accept the suggestion or anything else to write a different command.

With evil-company, that does not work because TAB is bound to company-select-next which does nothing in this case.

[jojojames]

Here's repro.

Keymap for session.

company-active-map is a variable defined in ‘company.el’.
Its value is shown below.

  This variable may be risky if used as a file-local variable.

Documentation:
Keymap that is enabled during an active completion.

Value:
(keymap
 (33554441 . company-select-previous)
 (backtab . company-select-previous)
 (4 . company-next-page)
 (21 . company-previous-page)
 (11 . company-select-previous-or-abort)
 (10 . company-select-next-or-abort)
 (16 . company-select-previous-or-abort)
 (14 . company-select-next-or-abort)
 (19 . company-search-candidates)
 (23 . company-show-location)
 (8 . company-show-doc-buffer)
 (f1 . company-show-doc-buffer)
 (9 . company-select-next)
 (tab . company-select-next)
 (13)
 (return)
 (up-mouse-3 . ignore)
 (up-mouse-1 . ignore)
 (mouse-3 . company-select-mouse)
 (mouse-1 . company-complete-mouse)
 (down-mouse-3 . ignore)
 (down-mouse-1 . ignore)
 (remap keymap
        (scroll-down-command . company-previous-page)
        (scroll-up-command . company-next-page))
 (up . company-select-previous-or-abort)
 (down . company-select-next-or-abort)
 (7 . company-abort)
 (27 keymap
     (107 . company-select-previous)
     (106 . company-select-next)
     (57 . company-complete-number)
     (56 . company-complete-number)
     (55 . company-complete-number)
     (54 . company-complete-number)
     (53 . company-complete-number)
     (52 . company-complete-number)
     (51 . company-complete-number)
     (50 . company-complete-number)
     (49 . company-complete-number)
     (48 . company-complete-number)
     (19 . company-filter-candidates)
     (112 . company-select-previous)
     (110 . company-select-next)
     (27 keymap
         (27 . company-abort))))

Repro # 1

company-backends is a variable defined in ‘company.el’.
Its value is (company-eshell-autosuggest)

company-frontends is a variable defined in ‘company.el’.
Its value is
(company-tng-frontend company-pseudo-tooltip-frontend company-echo-metadata-frontend company-quickhelp-frontend)

Type something, popup shows up with selection.

TAB works as expected, selection is selected and inserted.

;;

Repro # 2

company-backends is a variable defined in ‘company.el’.
Its value is (company-eshell-autosuggest)

(setq company-frontends '(company-tng-frontend company-preview-frontend))
company-frontends is a variable defined in ‘company.el’.
Its value is (company-tng-frontend company-preview-frontend)

Type something, popup shows up with selection.

TAB works as expected, selection is selected and inserted.

BUT you will see an extra preview inserted into the screen. (aka. the problem I described).

Can you tell me if this matches your expectation?

If it does, I don't think it's the TAB selection that is the problem (though it can be a factor in the solution/workaround).

It would be good to get exact values from company/etc to drive this forward.

[Ambrevar]

My case would correspond to repro # 2 except that I don't get any "popup".
Which is why pressing TAB does not do anything.

Also I don't have the "double preview problem".

[jojojames]

Sorry, the 'popup' in this case refers to the preview.

company-frontends is a variable defined in ‘company.el’.
Its value is (company-tng-frontend company-preview-frontend)

If you don't get a popup/preview, again, that doesn't seem like a 'TAB' problem.

The double preview only happens after you've completed/selected a selection but that can only happen if you saw the preview in the first place.

[Ambrevar]

I see the preview, but I can't complete so the double preview issue cannot happen.

[jojojames]

Can you post your related company-* values? company-frontends, company-backends, company-diag, etc?

[Ambrevar]

Emacs 25.2.2 (x86_64-unknown-linux-gnu) of 2017-06-28 on sst-p1307278fl
Company 0.9.4

company-backends: (company-eshell-autosuggest)

Used backend: company-eshell-autosuggest
Major mode: eshell-mode
Prefix: "company-diag\n"
Completions: none

company-frontends is a variable defined in ‘company.el’.
Its value is (company-preview-frontend)

company-backends is a variable defined in ‘company.el’.
Value: (company-eshell-autosuggest)

[jojojames]

Don't you want

company-frontends is a variable defined in ‘company.el’.
Its value is (company-tng-frontend company-preview-frontend)

company-tng-frontend also?

Also check your

company-minimum-prefix-length

[Ambrevar]

So if I add company-tng-frontend, it solves my issue but triggers the double-preview issue.
What's your solution for the latter?

If we enable company-tng in Evil-collection, it means that the user must enable the tng frontend? Should we automate this? Else we should document it.

[jojojames]

(defun +company-tng-configure-default ()
    "Applies the default configuration to enable `company-tng'."
    (add-to-list 'company-frontends 'company-tng-frontend)
    (defun +company-expand-yasnippet-or-return ()
      "Expand yas template or call RET normally."
      (interactive)
      (if (and company-active-map
               company-selection-changed
               (car company-candidates) ;; Making sure there are candidates.
               (or
                ;; `meghanada'
                (get-text-property 0 'meta (nth company-selection
                                                company-candidates))
                ;; `yasnippet'
                (get-text-property 0 'yas-template (nth company-selection
                                                        company-candidates))))
          (call-interactively #'company-complete-selection)
        (when company-selection-changed
          (company-complete-selection))
        (let ((company-active-map))
          (call-interactively (key-binding (kbd "RET"))))))

    (setq company-frontends
          '(company-tng-frontend
            company-pseudo-tooltip-unless-just-one-frontend
            company-echo-metadata-frontend
            company-preview-frontend
            company-quickhelp-frontend))

    ;; https://github.com/company-mode/company-mode/pull/706
    (defun +company-preview-frontend (command)
      "`company-mode' frontend showing the selection as if it had been inserted."
      (pcase command
        (`pre-command (company-preview-hide))
        (`post-command
         (unless (and company-selection-changed
                      (memq 'company-tng-frontend company-frontends))
           (company-preview-show-at-point (point)
                                          (nth company-selection
                                               company-candidates))))
        (`hide (company-preview-hide))))
    (advice-add #'company-preview-frontend :override #'+company-preview-frontend)

    (let ((keymap company-active-map))
      (define-key keymap [return] '+company-expand-yasnippet-or-return)
      (define-key keymap (kbd "RET") '+company-expand-yasnippet-or-return)
      (define-key keymap "\C-n" 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap [tab] 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap (kbd "TAB") 'company-select-next-if-tooltip-visible-or-complete-selection)
      (define-key keymap [S-tab] 'company-select-previous)
      (define-key keymap [S-iso-lefttab] 'company-select-previous)
      (define-key keymap [(shift tab)] 'company-select-previous)
      (define-key keymap [backtab] 'company-select-previous)
      (define-key keymap "\C-p" 'company-select-previous)
      (define-key keymap [backtab] 'company-select-previous)
      (define-key keymap (kbd "S-TAB") 'company-select-previous)))

You can give that a shot and reverse engineer the solution. It's in the advice to company-preview-frontend.

[jojojames]

Also, company-tng function already configures the frontend. I think you're resetting it somehow.

Here's the function definition.

(defun company-tng-configure-default ()
  "Applies the default configuration to enable company-tng."
  (setq company-frontends '(company-tng-frontend
                            company-pseudo-tooltip-frontend
                            company-echo-metadata-frontend))
  (let ((keymap company-active-map))
    (define-key keymap [return] nil)
    (define-key keymap (kbd "RET") nil)
    (define-key keymap [tab] 'company-select-next)
    (define-key keymap (kbd "TAB") 'company-select-next)
    (define-key keymap [backtab] 'company-select-previous)
    (define-key keymap (kbd "S-TAB") 'company-select-previous)))

[dieggsy]

@Ambrevar FWIW, I made some updates to https://github.com/dieggsy/company-eshell-autosuggest - it now comes with a minor-mode that locally overrides company (the map, frontends, backends, delay) for internal use, so this shouldn't be too much of a problem.

the minor mode is still a bit of a work in progress (I should probably expose the local company-active-map a bit more flexibly) but IMO it's an improvement over the previous method.

[Ambrevar]

Alright, this seems to be fixed then! Thanks!

[Ambrevar]

My bad, it does not seem to be fixed :(

[dieggsy]

@Ambrevar if it doesn't seem to be fixed, it could be on company-eshell-autosuggest's end - company-active-map is local to eshell's buffer now. In theory that means company-tng-configure-default shouldn't affect it at all.

[dieggsy]

Note that it's exposed through company-eshell-autosuggest-active-map

[jojojames]

I don't have any problems with eshell-autosuggest using my settings by the way. Again, I think @Ambrevar is hitting the double preview problem which means either monkey patching those functions (like I did) or not using the preview frontend with tng.

[dieggsy]

I don't have any problems with eshell-autosuggest using my settings by the way.

What do you mean by this? Are you saying eshell-autosuggest works for you, or that you'd rather it pick up your company-defaults? (There's an option for it to not override company-active-map, though I don't like or recommend it since IMO, tab should trigger shell completion (regardless of suggestion) and ret should trigger sending your input to th eshell (regardless of suggestion))

Again, I think @Ambrevar is hitting the double preview problem which means either monkey patching those functions (like I did) or not using the preview frontend with tng.

I'm just a bit confused as to how this is happening, since the new minor mode locally overrides front-end, back-end and active-map in an eshell hook, so as to just act like fish autosuggestions without external interference from company.

[jojojames]

I just meant I didn't have any problems with eshell-autosuggest before your changes. I haven't tried the new changes yet so can't comment on anything related to that.

[dieggsy]

Ah, got it. My biggest issue was the stated tab/ret behavior where company-active-map would take over these keys, which is decidedly not like fish.

[jojojames]

@Ambrevar Can you extrapolate on what you mean by this?

My bad, it does not seem to be fixed :(

While we have @dieggsy 's attention. :)

[Ambrevar]

Same issue as before: I see the preview but pressing does nothing.
If I add the tng frontend, then I run into the duplicate issue as we mentioned. I need to investigate more, between your solutions and what can be done on eshell-autosugggest's side.

[dieggsy]

@Ambrevar to clarify, are you using the minor mode, or not?

[Ambrevar]

Sorry, I had misunderstood your previous comment about the minor mode.

It works! I think it's a decent solution to the problem. (Maybe there is something better to be fixed upstream but I don't know company well enough.)

Typo in the docstring of the minor mode: company-eshell-autosuggest-selection-keys does not exist.

[dieggsy]

@Ambrevar i'll update that, it should say "...customizable by adding keys to company-eshell-autosuggest-active-map"

[Ambrevar]

I think it's safe to close this now. Thanks for the support!