Completion candidates for cider-find-var prompt
expez opened this issue · 26 comments
The prompt currently only contains thing-at-point. It would be nice if it also contained all other vars that are reachable from the current ns.
Try tab completion for cider-find-var. I'd argue the current behavior is way better than the alternative, although that's not apparent to ido users.
Can we get some optional ido-love up in here, then?
Can we get some optional ido-love up in here, then?
Unlikely - what makes sense in a language without namespaces (like Elisp), doesn't make a lot of sense in Clojure. There used to ido-friendly completion - first you'd see all the namespaces, you'd select one and then you'd see a list of all the vars. I killed this however, because it was super annoying to do 2 levels of selection all the time. cider-doc was also a concern - it operates on both Clojure and Java constructs, so using a Clojure-specific completion there didn't make sense and mixing Clojure and Java in an ido-friendly manner was pretty hard.
If somebody comes with a really nice idea how to organize this - I'll be willing to reconsider it.
Why didn't you just go with a list of fully qualified vars?
Why didn't you just go with a list of fully qualified vars?
Looked kind of ugly to me (plus I had some performance concerns for bigger projects and ido-flx).
Unlikely - what makes sense in a language without namespaces (like Elisp), doesn't make a lot of sense in Clojure. T
It makes sense as long as the names are unique. Non-unique elements could be qualified with namespace, or better ask for the namespace after a non-unique candidate has been selected.
BTW, adding INITIAL-INPUT is pretty non-standard emacs. It's much better to have the default choice. Ido also recognizes default choice and shifts it in front automatically. The initial input is unnecessary in all cases.
To be frank, this abomination with cider-read-from-minibuffer leads to rather confusing code and behavior. Cannot CIDER just use completing-read instead of trying to outsmart standard emacs? I would really appreciate if Cider completion worked as everywhere else in emacs, with or without IDO.
It makes sense as long as the names are unique. Non-unique elements could be qualified with namespace, or better ask for the namespace after a non-unique candidate has been selected.
That'd be a terrible idea, as adding namespaces to your project will constantly change your completion candidates. We should probably have everything fully-qualified and we should also have ns aliases in mind and interned vars in mind.
BTW, adding INITIAL-INPUT is pretty non-standard emacs. It's much better to have the default choice. Ido also recognizes default choice and shifts it in front automatically. The initial input is unnecessary in all cases.
I'm not quite sure what you mean or what's the problem right now.
To be frank, this abomination with cider-read-from-minibuffer leads to rather confusing code and behavior. Cannot CIDER just use completing-read instead of trying to outsmart standard emacs? I would really appreciate if Cider completion worked as everywhere else in emacs, with or without IDO.
This is pretty much impossible to avoid for the commands dealing with both Clojure and Java code. We'd be crazy do dump all the java classes and methods in a list... completing-read is great for simple non-hierarchical data, but things get messy in situations like ours (unless we flatten everything). I don't think we're doing something super crazy, although admittedly it's not super pretty either.
As I said before - if someone develops something super cool I'd happily consider using it.
The current behaviour is not without benefits.
Completely irrelevant but what is the font you are using in your last
screen capture @bbatsov?
Le 4 avr. 2015 12:04, "Bozhidar Batsov" notifications@github.com a écrit :
[image: screenshot 2015-04-04 13 57 32]
https://cloud.githubusercontent.com/assets/103882/6992391/7c14df00-dad3-11e4-946d-3613f5b9f57b.pngThe current behaviour is not without benefits.
—
Reply to this email directly or view it on GitHub
#1059 (comment)
.
Without INITIAL-INPUT but with a default selection:
With INITIAL-INPUT used as default selection:
I'm not quite sure what you mean or what's the problem right now.
The problem is that it's non-standard and that you now have to erase the content if you don't want the default suggestion before typing something else.
That'd be a terrible idea, as adding namespaces to your project will constantly change your completion candidate
In what sense do they change? There are simply more completions added. That some of those might not be unique anymore is true., but this is why the second level prompt for the namespace should be there.
BTW, adding INITIAL-INPUT is pretty non-standard emacs. It's much better to have the default choice. Ido also recognizes default choice and shifts it in front automatically. The initial input is unnecessary in all cases.
I'm not quite sure what you mean or what's the problem right now.
The problem is that the thing-at-point is inserted at prompt (aka INITIAL-INPUT in completing-read terminology) and you have to delete it if you don't want it (most commonly for me). In emacs it's customary to have the default choice which could be accessed with RET, and leave the minibuffer input clean.
This is pretty much impossible to avoid for the commands dealing with both Clojure and Java code. We'd be crazy do dump all the java classes and methods in a list...
Even if java objects/methods are restricted to only those imported in the current namespace?
As I said before - if someone develops something super cool I'd happily consider using it.
How about merging current completion mechanism with standard completing-read. That is, offer most likely list of candidates which is a subset of all candidates (all clojure, imported java) and on M-TAB get the completion popup as it's done currently.
You can also grow the completions dynamically. That is, have an original set of completions, and as user typed 2 letters request all of the remaining completions that start with those 2 letters.
Without INITIAL-INPUT but with a default selection:
@expez What is the completion engine in your first screenshot? Never seen this before. Feels like I am missing something big.
Seems it's ido-vertical.
@expez What is the completion engine in your first screenshot? Never seen this before. Feels like I am missing something big.
In what sense do they change? There are simply more completions added. That some of those might not be unique anymore is true., but this is why the second level prompt for the namespace should be there.
Two level prompt - one for ns, one for vars, right? We already had this and I killed it (although it actually had a few more levels - you select clojure, then core, then a var in core (and there was always an option to go back). Sounded good in theory but was super awkward to use.
The problem is that the thing-at-point is inserted at prompt (aka INITIAL-INPUT in completing-read terminology) and you have to delete it if you don't want it (most commonly for me). In emacs it's customary to have the default choice which could be accessed with RET, and leave the minibuffer input clean.
Ah, yeah. Now I get what you mean.
Even if java objects/methods are restricted to only those imported in the current namespace?
That might be more manageable, but will impair usability if you'd like to lookup something you're not yet using.
How about merging current completion mechanism with standard completing-read. That is, offer most likely list of candidates which is a subset of all candidates (all clojure, imported java) and on M-TAB get the completion popup as it's done currently.
Maybe.
At any rate - PRs welcome! This is pretty low on my todo list, so don't expect me doing significant changes in the foreseeable future.
Two level prompt - one for ns, one for vars, right?
No. The other way around. Select var, then if var is not unique prompt for namespace. I think this is how it used to work before for java methods - first ask for method, then ask for class if there were many matching classes. Was very handy.
No. The other way around. Select var, then if var is not unique prompt for namespace. I think this is how it used to work before for java methods - first ask for method, then ask for class if there were many matching classes. Was very handy.
This feature still exists for Java methods, but it's based on our current completion scheme - you select a method like .toString and then you'll be prompted for its class.
I don't wont to hide namespace information mostly because this is makes it harder to explore APIs. If we go down the route you suggest people will have to use the namespace browser if they want to find something they're not sure about.
Not sure if this is possible with how completing-read works, but what about a completing read where:
- The initial candidate list is the result of a normal completion with no prefix, i.e. core fns, everything in the current namespace, refers, etc. - and also all namespaces.
- If at any point the input becomes namespace-qualified (i.e. contains
/) we re-initiate completion to reset the candidate list to everything in that namespace. - If the user deletes the
/, we go back to the original candidate list.
This way, the candidate list is not massive and cluttered with namespace-qualified things, but it's still possible to complete namespace-qualified things. The downside being that said things are not in the candidate list from the start, so you have to know which namespace you're looking in - although we have cider-apropos for this kind of searching across namespaces :-).
FWIW regarding the prompt-on-duplicate idea - if I try to look up the documentation for something defined or referred in the current namespace, and something with the same name is defined in some completely-unrelated namespace, I would never want to be prompted. Though maybe the fact that there's always a "current namespace" at play feels intuitive to me, but not to most people, in which case a prompt by default would make sense.
edit: oh and definitely 👍 for default over initial input!
@cichli Your idea sounds reasonable, but I'm also uncertain if it's actually doable.
This gets pretty close:
(defun cider-completing-read (prompt default)
(let ((completion-extra-properties '(:annotation-function cider-annotate-symbol))
(completion-table (completion-table-dynamic #'cider-complete))
(prompt (format "%s (default %s): " prompt default)))
(completing-read prompt
completion-table
nil
nil
nil
'cider-minibuffer-history
default)))But it seems there's no way to IDO-ify this. ido-ubiquitous has an option ido-ubiquitous-allow-on-functional-collection that enables it when using a function for the COLLECTION argument to completing-read like the above, but it does not call the function more than once. Additionally, ido-ubiquitous unconditionally will not use IDO when completion-extra-properties is set - see here.
So perhaps if we use something like the above, but ensure that it does not bind completion-extra-properties when cider-annotate-completion-candidates is false, then we could advise users of ido-ubiquitous to set ido-ubiquitous-allow-on-functional-collection to t. It's then still possible to fallback to the standard completing-read behaviour using C-f (as is always true in IDO) for non-IDO-style completion of namespace-qualified things.
Can you expand on this a bit with some examples? I'm not quite sure what the problem is and what the proposed solution is.
When discussing the proposed mechanics of the xref package (for Emacs 25), I spent some time thinking how identifier completion could work for Clojure, and reached, very closely, what @cichli describes in #1059 (comment).
If there are some problems with completing-read impeding that approach, we should work to fix those in the core.
But it seems there's no way to IDO-ify this.
Maybe not right now, but it certainly can be made to work in the Ido interface. Someone just needs to implement it. Does it work with Ivy or Helm?
Using ido-ubiquitous-allow-on-functional-collection won't work, because we'll have the exact kind of "more complex behavior" its docstring warns against.
@bbatsov by using completing-read we can make this open to configuration, so people can use IDO/Ivy/Helm as they please. The problem is, none of those three currently support (except ido-ubiquitous, partially) using a completion function as the collection argument to completing-read. ido-ubiquitous also explicitly falls back to standard completing-read if completion-extra-properties is set, but this is merely because support for it is not yet implemented, rather than because it cannot be done - see DarwinAwardWinner/ido-completing-read-plus#8.
@dgutov I've experimented a bit at improving the ido-ubiquitous support for functional collections, but haven't come up with anything that I think is worthy of a PR yet. Here's one approach (requires a slight modification to ido-ubiquitous to add and set the ido-ubiquitous-current-collection and ido-ubiquitous-current-predicate vars when completing-read-ido-ubiquitous is called):
(defun ido-ubiquitous-refresh-completions ()
(interactive)
(when (ido-active)
(let* ((prefix (minibuffer-completion-contents))
(completions (all-completions prefix
ido-ubiquitous-current-collection
ido-ubiquitous-current-predicate)))
(when completions
(setq ido-matches completions)
(ido-restrict-to-matches)))))Adding a keybind for this to ido-completion-map allows for this kind of usage (here it is bound to <tab>, and ido-take-first-match to C-<tab>:
I guess it would also be possible to call the completion function every time the minibuffer contents are altered (without clearing the minibuffer contents like ido-restrict-to-matches does), but that may not be feasible performance-wise.
@cichli Adding tab binding is a hack. Ido is supposed to refresh the results automatically.
I guess it would also be possible to call the completion function every time the minibuffer contents are altered (without clearing the minibuffer contents like ido-restrict-to-matches does), but that may not be feasible performance-wise.
I think it should work well enough (company-capf is an evidence of that). Though a proper solution will likely involve extending ido-completing-read to support completions functions, and that function is in Emacs core.
On the other hand, using a completion function will likely conflict with Ido's flex matching. But there's nothing we can do about that, it seems.
In any case, try icomplete-mode instead of ido-ubiquitous. It's not Ido, but it handles completion functions fine.