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

Put <return> as company-auto-complete-chars #530

Closed
magicdirac opened this issue Jun 4, 2016 · 27 comments
Closed

Put <return> as company-auto-complete-chars #530

magicdirac opened this issue Jun 4, 2016 · 27 comments
Labels

Comments

@magicdirac
Copy link

magicdirac commented Jun 4, 2016

How to put <return> as one of the company-auto-complete-chars?
so that I can unbind from company-active-map

(require 'bind-key)
(with-eval-after-load 'company
    (unbind-key "RET" company-active-map) 
    (unbind-key "<return>" company-active-map)
)

The benefit is that I will get normal <return> functionality when I press <return>, unless I have interacted with company explicitly, then it will insert the choice I chose with this setting

(setq company-auto-complete 'company-explicit-action-p)
@dgutov
Copy link
Member

dgutov commented Jun 4, 2016

company-auto-complete-chars ... can also be a function, which is called with the new input and should return non-nil if company should auto-complete.

This seems like the best available option for that.

@magicdirac
Copy link
Author

Hi, can you help me how to do it with a function.

I try to add as a character to company-auto-complete-chars, that simplify does not work.
(add-to-list 'company-auto-complete-chars ?\r)

@dgutov
Copy link
Member

dgutov commented Jun 5, 2016

Something like this

(defun my-return-p (input)
  (eq ?\n (string-to-char input)))

(setq company-auto-complete-chars #'my-return-p)

But it seems to have a fatal problem: it inserts a newline afterwards. Which is expected for company-auto-complete in the normal case, but not here. Probably.

@dgutov dgutov added the question label Jun 5, 2016
@magicdirac
Copy link
Author

you are so cool, it works.
Thank you very much!

@dgutov
Copy link
Member

dgutov commented Jun 5, 2016

OK, if that's what you wanted.

@dgutov dgutov closed this as completed Jun 5, 2016
@unhammer
Copy link

unhammer commented Jun 16, 2016

Is there a simple way to get RET/ to complete only on explicit-action-p but without inserting a newline?

@dgutov
Copy link
Member

dgutov commented Jun 16, 2016

@unhammer You can define a new command and bind it.

@magicdirac
Copy link
Author

magicdirac commented Jun 16, 2016

 (define-key company-active-map (kbd "<return>")
    #'(lambda () (interactive)
        (progn (if (company-explicit-action-p)
                   (company-complete)
                 (call-interactively
                  (lookup-key (current-global-map)
                              (kbd "RET")))
                 ))))
  (define-key company-active-map (kbd "RET")
    #'(lambda () (interactive)
        (progn (if (company-explicit-action-p)
                   (company-complete)
                 (call-interactively
                  (lookup-key (current-global-map)
                              (kbd "RET")))
                 ))))

EDIT: update to a better version below.

  (defun my-company-active-return ()
    (interactive)
    (if (company-explicit-action-p)
        (company-complete)
      (call-interactively
         (or (key-binding (this-command-keys))
             (key-binding (kbd "RET")))
    )))
(dedfine-key company-active-map (kbd "<return>") #'my-company-active-return)
(define-key company-active-map (kbd "RET") #'my-company-active-return)

@dgutov
Copy link
Member

dgutov commented Jun 16, 2016

@magicdirac Good example. Notes:

  • No need to quote the lambdas.
  • You should define a named function and bind it, so that you don't have the same anonymous definition twice.

@magicdirac
Copy link
Author

magicdirac commented Jun 17, 2016

                  (lookup-key (current-global-map)
                              (kbd "<return>")))

The first one I tried to look up "<return>", it does not work. I do not know why.
And then I changed to "RET"
That's why the two functions are now identical.

@dgutov
Copy link
Member

dgutov commented Jun 17, 2016

Those are different keys. <returns> maps to RET when unbound, but not vice versa.

The global map likely has only a binding for RET.

@unhammer
Copy link

unhammer commented Jun 17, 2016

If RET is bound by a minor mode, maybe

(let ((company-mode nil))
             (or (key-binding (kbd "<return>"))
                   (key-binding (kbd "RET")))
        )

is more general.

Ideally, one could go from last-command-event directly to whatever keybinding it has, but I couldn't get that to work; it seems binding to "" gives the symbol 'return while binding to "RET" gives 13, no idea if there's a function that can translate into whatever would be called by the same key. But this is way beyond my use-case :)

@dgutov
Copy link
Member

dgutov commented Jun 17, 2016

If RET is bound by a minor mode, maybe ... is more general

I think so.

Ideally, one could go from last-command-event directly to whatever keybinding it has, but I couldn't get that to work

Have you tried (key-binding (this-command-keys))?

@magicdirac
Copy link
Author

magicdirac commented Jun 17, 2016

(key-binding (this-command-keys))
This would return nil if <return> is bound in company-active-map.
However, if you unbind <return> from company-active-map, then it will translate to RET and everything works fine.
Maybe we should not define <return> at all. I do not see any advantages to have a separate <return> binding, as all global modes translate it to RET

@dgutov
Copy link
Member

dgutov commented Jun 17, 2016

We've had the <return> binding because autopair had it, but apparently not anymore.

So OK, let's try removing it.

Too bad we can't do so with tab as well, because org-mode binds it.

dgutov added a commit that referenced this issue Jun 17, 2016
Autopair doesn't bind it anymore.  Org doesn't seem to either.
@ackalker
Copy link

@dgutov Please revert this change, it breaks documented and expected behaviour (http://company-mode.github.io/ Usage).
Using Emacs 24.5.1 in GUI mode, with its built-in electric-pair-mode, I can no longer select a completion candidate using <return> in either SLIME or Geiser REPLs (though it still works in IELM), I now have to use <kp-enter> (Enter key on numeric keypad) to select a completion, which is much too far from the home row for my liking.

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

I can no longer select a completion candidate using in either SLIME or Geiser REPLs

Do you know why that happens?

@ackalker
Copy link

ackalker commented Jun 20, 2016

@dgutov Ah sorry for not being clear: when I hit <return> on a selection, the (usually incomplete) sexp gets sent to the REPL instead of inserting the candidate.
According to C-h k <return>:

in Geiser, geiser-repl--maybe-send is bound to <return>, RET,

in SLIME, slime-repl-return is bound to <return>, RET, <menu-bar> <REPL> <Send Input>.

in IELM, ielm-return is bound only to RET (mapped from <return> I assume).

Yeah, I know, this whole <return> vs. RET (and <tab> vs TAB) handling is a mess...

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

Could you please report this issue to Geiser and SLIME, and ask them not to bind return?

Unless they have convincing reasons to do this, I'd rather keep this change.

@ackalker
Copy link

@dgutov I'm afraid that's going to be tough (especially in the case of SLIME, which will have to consider breakage of expected behaviour of lots of heavily invested users), as I'm just another user. Why not hold off on the change until the other packages have dropped their 'double' bindings? Or are you trying to force their hand?

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

Why not hold off on the change until the other packages have dropped their 'double' bindings?

When do you estimate that might happen? And what would cause them to?

Or are you trying to force their hand?

I suppose I am, now that you brought the problem to my attention.

Anyway, could you please report the issues, and we'll see how it goes from here, depending on the responses.

@ackalker
Copy link

I'm sorry, but I think you're fighting an uphill struggle here, it takes only a single (minor) mode out of the dozens out there to bind <return> and break completion selection.
Why not control the binding of <return> with a customizable variable with default value t?

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

I'm sorry, but I think you're fighting an uphill struggle here

That's not the way to convince me.

Why not control the binding of with a customizable variable with default value t?

Any user can re-add the binding in their own init script even without such a variable.

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

Actually, maybe I can avoid having this argument by applying the syntax table via an overlay, and fixing an old issue along with it.

Please stand by.

@dgutov
Copy link
Member

dgutov commented Jun 21, 2016

applying the syntax table via an overlay

It doesn't seem to help with the problem in question, sorry.

On the brighter side, auto-complete also seems to have stopped binding [return] over a year ago: auto-complete/auto-complete@81fd921

So the hill can't be too high.

dgutov added a commit that referenced this issue Jun 23, 2016
@dgutov
Copy link
Member

dgutov commented Jun 23, 2016

I've reverted that change because of (EDIT) #543, sorry @magicdirac.

Going against Org this way would indeed be difficult.

@magicdirac
Copy link
Author

magicdirac commented Jun 27, 2016

@dgutov cool, brother. I will then keep <return> re-binding.

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

No branches or pull requests

4 participants