Skip to content

Completion metadata #15

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

Merged
merged 8 commits into from
Feb 28, 2015
Merged

Conversation

cichli
Copy link
Member

@cichli cichli commented Feb 25, 2015

cc @bbatsov @gtrak

As discussed in #12. This is a breaking change but it would be easy to keep the old behaviour and add a new function for this.

See the tests for more examples but completions now returns a sequence that looks like this:

({:candidate "*1" :ns cljs.core :type :var}
 {:candidate "-add-watch" :ns cljs.core :type :protocol-fn}
 {:candidate "aget" :ns cljs.core :type :fn}
 {:candidate "doseq" :ns cljs.core :type :macro}
 {:candidate "IdGenerator" :ns goog.ui.IdGenerator :type :import}
 {:candidate "TestRecord" :ns cljs-tooling.test-ns :type :record}
 {:candidate "TransientArrayMap" :ns cljs.core :type :type})

The possible values of :type are:

:special-form
:ns
:fn
:protocol-fn
:record
:type
:protocol
:var
:macro
:import

If there are multiple candidates with the same value of :candidate (i.e. any of the fns in cljs.core that wrap macros with the same name), then only one will be used - the one that would be returned by an info call for the value of :candidate (i.e. order of preference is the same as the list above).

Not sure if we should force the client to account for all of the possible types or collapse them down a bit?

@bbatsov
Copy link
Member

bbatsov commented Feb 26, 2015

Great work! Ideally @alexander-yakushev would add the same behaviour to compliment soon as well!

@alexander-yakushev
Copy link
Member

Can anyone tell me the different between :candidate and :name?

@cichli
Copy link
Member Author

cichli commented Feb 26, 2015

Sorry - didn't include enough examples to make it clear :-). :candidate can contextually be fully-qualified, e.g. if prefix is cljs.core.async.macros/g then completions will look like:

({:candidate "cljs.core.async.macros/go" :ns cljs.core.async.macros :name cljs.core.async.macros/go :type :macro}
 {:candidate "cljs.core.async.macros/go-loop" :ns cljs.core.async.macros :name cljs.core.async.macros/go-loop :type :macro})

and for prefix dose:

({:candidate "doseq" :ns cljs.core :name cljs.core/doseq :type :macro})

So basically the values of :candidate are what the editor will actually use for completion, whereas :name is just the non-namespace-qualified namespace-qualified name.

@cichli
Copy link
Member Author

cichli commented Feb 26, 2015

Happy to remove :name if it's not that useful btw.

@bbatsov
Copy link
Member

bbatsov commented Feb 26, 2015

Seems your original examples was wrong - as :name and :candidate where reversed there, which looked wrong (but I didn't notice until @alexander-yakushev mentioned it). I think it the current format is good.

@alexander-yakushev
Copy link
Member

Don't worry, I'm asking this for myself because @bbatsov won't give me a break :D. So Bozhidar is this something you want from Compliment too?

@bbatsov
Copy link
Member

bbatsov commented Feb 26, 2015

@alexander-yakushev Yes. I think I created a ticket for this half a year ago. It will make it possible for us to have richer completion metadata (e.g. in company, ac, etc). See alexander-yakushev/compliment#16

@cichli
Copy link
Member Author

cichli commented Feb 26, 2015

Yep, well spotted - I lied about :name, it's not consistent at all right now 🤦. I think it should always be namespace-qualified (as it is currently for macros only). Will sort it tonight.

@alexander-yakushev
Copy link
Member

@bbatsov I specifically mean :name and :candidate distinction.

@cichli
Copy link
Member Author

cichli commented Feb 26, 2015

We can probably just omit :name entirely unless the client intends to actually use it. info ops already work fine on completion candidates regardless of whether they're ns-qualified or not.

@cichli cichli force-pushed the completion-metadata branch from a8b0fc4 to c64754e Compare February 26, 2015 18:02
@cichli
Copy link
Member Author

cichli commented Feb 26, 2015

Okay, patch updated with :name removed and some other bits:

  • test case for fns that shadow macros in the same ns
  • :special type renamed to :special-form to match compliment
  • some docstring tweaks

@bbatsov
Copy link
Member

bbatsov commented Feb 27, 2015

👍 Let's merge this!

P.S. While you're on the subject of small tweaks - you might improve the README as well. :-)

@gtrak
Copy link
Contributor

gtrak commented Feb 27, 2015

Will take a look tomorrow. Thanks!

On Fri Feb 27 2015 at 2:28:34 PM Bozhidar Batsov [email protected]
wrote:

[image: 👍] Let's merge this!

P.S. While you're on the subject of small tweaks - you might improve the
README as well. :-)


Reply to this email directly or view it on GitHub
https://github.com/gtrak/cljs-tooling/pull/15#issuecomment-76456072.

@cichli cichli force-pushed the completion-metadata branch from c64754e to 161506f Compare February 27, 2015 20:02
gtrak added a commit that referenced this pull request Feb 28, 2015
@gtrak gtrak merged commit 3f9d238 into clojure-emacs:master Feb 28, 2015
@gtrak
Copy link
Contributor

gtrak commented Feb 28, 2015

Great work!

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

Successfully merging this pull request may close these issues.

4 participants