Conflict between company-tng and company-preview-frontend

[Ambrevar]

I use company-eshell-autosuggest which uses company-preview-frontend in Eshell to display commandline suggestions "à-la fish-shell".

TAB is normally used to confirm the suggestion. It is bound to company-complete-common.

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.

This could be considered an upstream issue but maybe some of you know better. Any opinion on this?

[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?

[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))

[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!