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

helm completion in Sly #2

Closed
thierryvolpiatto opened this issue Oct 20, 2019 · 14 comments
Closed

helm completion in Sly #2

thierryvolpiatto opened this issue Oct 20, 2019 · 14 comments

Comments

@thierryvolpiatto
Copy link
Member

Just for info.
In helm now to have native sly completion:

  1. (customize-set-variable 'helm-completion-style 'emacs)
  2. (add-to-list 'completion-styles 'backend)
  3. Start Sly
  4. M-x sly-symbol-completion-mode => To disable it (you can do this in config by hook).
@Ambrevar
Copy link
Member

Ambrevar commented Sep 9, 2020

In Emacs 27 using (add-hook 'sly-mrepl-hook #'helm-sly-disable-internal-completion) hangs Emacs (cannot even C-g).

(defun helm-sly-disable-internal-completion ()
  "Disable SLY own's completion system, e.g. to use Helm instead.
This is mostly useful when added to `sly-mrepl-hook'."
  (sly-symbol-completion-mode -1))
  • (customize-set-variable 'helm-completion-style 'emacs) does not change anything.
  • Adding backend to completion-styles does not change anything.
  • Calling (sly-symbol-completion-mode -1) then pressing M-tab after (uiop: hangs Emacs.

@thierryvolpiatto Any idea?

1 similar comment
@Ambrevar
Copy link
Member

Ambrevar commented Sep 9, 2020

In Emacs 27 using (add-hook 'sly-mrepl-hook #'helm-sly-disable-internal-completion) hangs Emacs (cannot even C-g).

(defun helm-sly-disable-internal-completion ()
  "Disable SLY own's completion system, e.g. to use Helm instead.
This is mostly useful when added to `sly-mrepl-hook'."
  (sly-symbol-completion-mode -1))
  • (customize-set-variable 'helm-completion-style 'emacs) does not change anything.
  • Adding backend to completion-styles does not change anything.
  • Calling (sly-symbol-completion-mode -1) then pressing M-tab after (uiop: hangs Emacs.

@thierryvolpiatto Any idea?

@Ambrevar
Copy link
Member

Ambrevar commented Sep 9, 2020

Cc @joaotavora

@joaotavora
Copy link

It's doesn't really hang Emacs according to this report, at least: joaotavora/sly#303, which this seems to be a duplicate of.

@Ambrevar
Copy link
Member

Ambrevar commented Sep 9, 2020 via email

@joaotavora
Copy link

joaotavora commented Sep 9, 2020

Any workaround at the moment?

Stop using Helm?

Heheh, just kidding. I don't know. You can try reverting that SLY commit or downgrading SLY. SLY works fine with Emacs, which is what it is designed to do, but Helm is a fairly complicated program which I'm not familiar with, so debugging asynchronous code with it enabled is hard.

Especially here, where I would have to know how it connects to Emacs's completion system and what how it expects that completion system to behave. Only @thierryvolpiatto has that knowledge. Presumably, SLY, who is also also connecting to that completion system "from the other side" is making it behave in some way that Helm doesn't expect. Should it expect it? Should it not? That is the hard work that someone (:tm:) must do.

@Ambrevar
Copy link
Member

Ambrevar commented Sep 9, 2020 via email

@joaotavora
Copy link

Yes, company seems to connects to the completion system "correctly", and then helm probably connects to company instead.

@thierryvolpiatto
Copy link
Member Author

thierryvolpiatto commented Sep 9, 2020 via email

@thierryvolpiatto
Copy link
Member Author

thierryvolpiatto commented Sep 9, 2020 via email

@joaotavora
Copy link

Stop using Helm?

Or stop using Sly ;-)

Alright, let's bury the passive-aggressive joke hammer. I'm debugging this now, and I hope you can assist with anything you know about Helm's code.

@joaotavora
Copy link

Here, I cooked up a really hacky patch to helm-mode that fixes this. Just an idea, not an actual patch. I have really very little idea about Helm's code, but I suspected while-no-input anyway. Well, since you skip while-no-input for TRAMP maybe you can also skip it in more situations, such as when dealing with completion-at-point-functions which is where SLY hooks its completion function.

diff --git a/helm-mode.el b/helm-mode.el
index bcf02d5a..ef771568 100644
--- a/helm-mode.el
+++ b/helm-mode.el
@@ -1732,6 +1732,7 @@ is non-nil."
     (setq helm-saved-selection (helm-get-selection nil 'withprop))))
 
 ;; Completion-in-region-function
+(defvar joaot/helm-original-buffer nil)
 
 (defun helm--completion-in-region (origfun start end collection &optional predicate)
   "Helm replacement of `completion--in-region'.
@@ -1804,6 +1805,7 @@ Can be used for `completion-in-region-function' by advicing it with an
                                   ;; Assume that when `afun' and `predicate' are null
                                   ;; we are in filename completion.
                                   (and (null afun) (null predicate))))
+                 (joaot/helm-original-buffer (current-buffer))
                  ;; `completion-all-completions' store the base-size in the last `cdr',
                  ;; so data looks like this: '(a b c d . 0) and (last data) == (d . 0).
                  base-size
diff --git a/helm.el b/helm.el
index 4f749472..91b68a8a 100644
--- a/helm.el
+++ b/helm.el
@@ -4610,10 +4610,16 @@ emacs-27 to provide such scoring in emacs<27."
                                              'helm-multiline t)))))
 
 (defmacro helm--maybe-use-while-no-input (&rest body)
-  "Wrap BODY in `helm-while-no-input' unless initializing a remote connection."
+  "Wrap BODY in `helm-while-no-input' unless initializing a remote connection.
+Or talking to SLY."
   `(progn
-     (if (and (file-remote-p helm-pattern)
-              (not (file-remote-p helm-pattern nil t)))
+     (if (or (and (bufferp joaot/helm-original-buffer)
+                  (with-current-buffer joaot/helm-original-buffer
+                    (bound-and-true-p sly-mode)))
+             ;; maybe check for the 'backend' completion style here
+             ;; instead?
+             (and (file-remote-p helm-pattern)
+                  (not (file-remote-p helm-pattern nil t))))
          ;; Tramp will ask for passwd, don't use `helm-while-no-input'.
          ,@body
        (helm-log "Using here `helm-while-no-input'")

@joaotavora
Copy link

Here's a much simpler patch to Helm. If Helm likes to play with inhibit-quit for process-based completion functions, which is how I interpret this code, then it might as well do that for completion-at-point-functions, since processes may be involved there, too:

diff --git a/helm.el b/helm.el
index 4f749472..ba2c01ee 100644
--- a/helm.el
+++ b/helm.el
@@ -3979,7 +3979,7 @@ WARNING: Do not use this mode yourself, it is internal to Helm."
   (let* ((candidate-fn (assoc-default 'candidates source))
          (candidate-proc (assoc-default 'candidates-process source))
          ;; See comment in helm-get-cached-candidates (Issue 2113).
-         (inhibit-quit candidate-proc)
+         (inhibit-quit t)
          cfn-error
          (notify-error
           (lambda (&optional e)

@Ambrevar
Copy link
Member

Fixed with joaotavora/sly@070b1db.

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

No branches or pull requests

3 participants