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

company-yasnippet and returning "" for prefix #641

Closed
jojojames opened this issue Feb 13, 2017 · 7 comments
Closed

company-yasnippet and returning "" for prefix #641

jojojames opened this issue Feb 13, 2017 · 7 comments
Labels

Comments

@jojojames
Copy link
Contributor

(defun company-yasnippet (command &optional arg &rest ignore)
  "`company-mode' backend for `yasnippet'."
  (interactive (list 'interactive))
  (cl-case command
    (interactive (company-begin-backend 'company-yasnippet))
    (prefix
     ;; Should probably use `yas--current-key', but that's bound to be slower.
     ;; How many trigger keys start with non-symbol characters anyway?
     (and (bound-and-true-p yas-minor-mode)
          (company-grab-symbol)))
    (annotation
     (concat
      (unless company-tooltip-align-annotations " -> ")
      (get-text-property 0 'yas-annotation arg)))
    (candidates (company-yasnippet--candidates arg))
    (no-cache t)
    (post-completion
     (let ((template (get-text-property 0 'yas-template arg))
           (prefix-offset (get-text-property 0 'yas-prefix-offset arg)))
       (yas-expand-snippet (yas--template-content template)
                           (- (point) (length arg) prefix-offset)
                           (point)
                           (yas--template-expand-env template))))))

It looks like for 'prefix case, company-yasnippet always returns at least "".

This is somewhat painful when setting minimum-prefix-length to 1 and after something like "." completion.

this.| triggers company-yasnippet because the prefix will be "" but I'm not sure I'll ever want yasnippet to trigger in that case.

Changing the function to be,

  (defun company-yasnippet (command &optional arg &rest ignore)
    "`company-mode' backend for `yasnippet'."
    (interactive (list 'interactive))
    (cl-case command
      (interactive (company-begin-backend 'company-yasnippet))
      (prefix
       ;; Should probably use `yas--current-key', but that's bound to be slower.
       ;; How many trigger keys start with non-symbol characters anyway?
       (and (bound-and-true-p yas-minor-mode)
            (let ((company-symbol (company-grab-symbol)))
              (if (string-equal company-symbol "")
                  nil
                company-symbol))))
      (annotation
       (concat
        (unless company-tooltip-align-annotations " -> ")
        (get-text-property 0 'yas-annotation arg)))
      (candidates (company-yasnippet--candidates arg))
      (no-cache t)
      (post-completion
       (let ((template (get-text-property 0 'yas-template arg))
             (prefix-offset (get-text-property 0 'yas-prefix-offset arg)))
         (yas-expand-snippet (yas--template-content template)
                             (- (point) (length arg) prefix-offset)
                             (point)
                             (yas--template-expand-env template))))))

lets me not see any yasnippet completions for the 'this.' case.

I'm not sure if this is a good approach though since I don't use manual completion and there may be a chance we want the "" prefix back from yasnippet?

This is, at the very least, a feature request to make this behavior configurable. I can do a pull request if this approach is acceptable.

@dgutov
Copy link
Member

dgutov commented Feb 15, 2017

setting minimum-prefix-length to 1

this.| triggers company-yasnippet because the prefix will be ""

Why does this happen? The length of "" is 0, and that's lower than 1.

@jojojames
Copy link
Contributor Author

I think completion triggers after typing the "." but it's not included in the prefix.

@dgutov
Copy link
Member

dgutov commented Feb 15, 2017

That's probably because you have another backend in the same group that triggers it.

@jojojames
Copy link
Contributor Author

Yup! It's part of a grouped backend.

@dgutov
Copy link
Member

dgutov commented Feb 15, 2017

We could somehow filter out backends that don't report a long enough prefix inside company--multi-backend-adapter-candidates, but that would require maintaining separate caches for all backends in the group. Or getting rid of the cache altogether, leaving it up for each individual backend.

That would solve the problem in a general way.

As far as your proposed change goes, it's too personal, I think. You can just as well add an advice to it.

@jojojames
Copy link
Contributor Author

Thanks! I'll go with the advice then.

dgutov added a commit that referenced this issue Oct 4, 2023
…eparately

This should help with
#1398,
#641,
#553,
#485,
#1147,
#840

With some of the above already closed, and some describing more serious
scenarios than others.

Anyway, this is a halfway step to dropping framework-level caching (or moving it to
backends).  For now this seems like an adequate solution, given that the most
"heavy" backend these days is `company-capf`, which disables caching already.
Though `company-dabbrev-code` is a close next.
dgutov added a commit that referenced this issue Oct 9, 2023
…eparately (#1405)

* company--multi-backend-adapter: Consider min prefix in 'candidates' separately

This should help with
#1398,
#641,
#553,
#485,
#1147,
#840

With some of the above already closed, and some describing more serious
scenarios than others.

Anyway, this is a halfway step to dropping framework-level caching (or moving it to
backends).  For now this seems like an adequate solution (resetting the cache
sometimes when the new chars are typed), given that the most "heavy" backend
these days is `company-capf`, which disables caching already.
Though `company-dabbrev-code` is a close next.

Uses new semi-private backend action called 'set-min-prefix', to keep the merging function's backward compatibility, for the sake of external callers.
@dgutov
Copy link
Member

dgutov commented Oct 9, 2023

Check out the latest master, the behavior should be improved.

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

2 participants