Add autocompletion with Zotero snippets to ACE #71
Conversation
By default basic autocompletion is now enabled which can be triggered by ctrl+space. The new option allows the user to switch for live autocompletion. Complete snippets and zotero functions.
zuphilip
changed the title
|
Here is a short demo:
I am happy with this now. Any feedback @dstillman, @adam3smith? |
| }, | ||
| { | ||
| content: "var item = new Zotero.Item(${1:itemType});", | ||
| name: "new Zotero.Item" |
There was a problem hiding this comment.
I think it's best to actually treat these as autocompletes, not shortcuts, so it shouldn't autocomplete to something before what you've actually typed (var item = in this case). The full string could be a different snippet.
| var ZoteroCompleter = { | ||
| getCompletions: function(editor, session, pos, prefix, callback) { | ||
| callback(null, [ | ||
| {value: "ZU.cleanAuthor", score: 1000, meta: "Zotero Utility function"}, |
There was a problem hiding this comment.
Is this meta required? These seem a bit redundant. (If they need to stay, Utility should be lowercased.)
There was a problem hiding this comment.
Not sure, but it shows up in the suggestion selector, e.g.
| ]); | ||
| } | ||
| } | ||
| var langTools = require("ace/ext/language_tools"); |
There was a problem hiding this comment.
Be sure to document the source of that file in the commit.
| <hbox id="editor-external-box" align="center"> | ||
| <checkbox id="checkbox-editor-external"/> | ||
| <label class="label-metadata" value="&scaffold.editor.external.label;" control="checkbox-editor-external"/> | ||
| <checkbox id="checkbox-live-autocompletion"/> | ||
| <label class="label-metadata" value="&scaffold.editor.liveAutocompletion.label;"/> |
There was a problem hiding this comment.
I think this can just be on rather than configurable. If someone doesn't want it, they're probably not editing in ACE to begin with.
There was a problem hiding this comment.
There is also the basic autocompletion which is invoked with ctrl+space (at least in windows) which is always activated.
The live autocompletion is IMO sometimes a little disturbing, e.g. I type a in the editor and get suggestions for ZU.cleanAuthor, Z.monitorDOMChanges, ZU.xpath etc. Try it out yourself at http://plnkr.co/edit/Ep3bVPjjkm1laImq443t?p=preview .
Moreover, any feedback from @adam3smith would be appreciated about this.
There was a problem hiding this comment.
Yes, I've tried it. If we're going to have it at all, I think it's more awkward to have to turn it on and off.
There was a problem hiding this comment.
But also that just sounds like bad autocomplete. It shouldn't trigger ZU.cleanAuthor when you type a. Can we make it left-bound?
There was a problem hiding this comment.
Make it left-bound or wait for at least 2-3 characters could work better, but I haven't found anything like this (but it is very hard to find information at all).
The live autocompletion is also disabled in the official demo https://ace.c9.io/build/kitchen-sink.html
There was a problem hiding this comment.
Minimum length is a good idea. Assuming we can get to them from the outside, I suspect we can monkey patch getCompletionPrefix or retrievePrecedingIdentifier to do that.
There was a problem hiding this comment.
They may even take such a patch into their official repo, which we could then simply reuse the official way. However, that is not something I can do currently.
There was a problem hiding this comment.
That's fine — I can look into this. But I think we should assume we'll have less annoying autocomplete and not bother with a checkbox for it.
| @@ -0,0 +1,47 @@ | |||
| These special Zotero functions are most frequent used in translator coding, for autocompletion try also ctrl+space during writing. | |||
There was a problem hiding this comment.
These are the most common functions used in Zotero translators.
Does the second part only apply if the live option is off?
| var snippetManager = ace.require("ace/snippets").snippetManager; | ||
| snippets = [{ | ||
| content: "for (let i=0; i<${1:arrayName}.length; i++) {\n\t${2:// continue here}\n}\n", | ||
| name: "simple for loop" |
There was a problem hiding this comment.
If we're going to bother with stuff like this, probably better to default to for...of, with two placeholders.
We should first merge #70.
This is a proof-of-concept for #69 for opening a round of feedback.