-
Notifications
You must be signed in to change notification settings - Fork 240
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
enable autocompletion in tutor Ace instances #9
Conversation
78fe8dd
to
f3d810b
Compare
f7d1c48
to
390af0a
Compare
I think this PR should be good to go -- @jjallaire, can you review + merge if all looks well? We can iterate on specific behaviors on master if there's anything you feel can / should be tweaked. |
|
||
document.addEventListener("keydown", function(event) { | ||
|
||
// TODO: find more appropriate place for one-time initialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we okay w/ this or is there still an issue outstanding here?
@@ -0,0 +1,291 @@ | |||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just .gitignore this file
I got this error in the R console when typing within the autocompletion sandbox Rmd:
|
I'm not seeing completions for the setup chunk (i.e. if I type |
Looks great! I'll merge it now. One thing I noticed is that we don't get automatically triggered completions for |
It should be relatively straightforward -- I'll give that a shot! |
I'm also wondering if we should leave off explicit S3 methods for e.g. |
We could, but note that those methods are indeed exported + visible on the search path (ie, they're used both in S3 dispatch and are exported as though they are free functions). We also show those completions in the RStudio IDE and nobody has complained thus far. |
Okay let's leave as-is then!
…On Fri, Jan 27, 2017 at 4:52 PM Kevin Ushey ***@***.***> wrote:
We could, but note that those methods are indeed exported + visible on the
search path (ie, they're used both in S3 dispatch and are exported as
though they are free functions). We also show those completions in the
RStudio IDE and nobody has complained thus far.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGXx13vTJdYuwv_6EZE7Z39u-KUMAcNks5rWmcugaJpZM4LuMsF>
.
|
A couple more small things to consider:
1) We currently show the "R" scope on the far right but as far as I can see
it's always "R" and never anything else. Seems like we could drop this.
2) The overall width could I think be trimmed to accommodate the contents
better (sort of like we do in RStudio).
Hopefully both of the above could be accomplished w/ just CSS changes
…On Fri, Jan 27, 2017 at 4:55 PM, JJ Allaire ***@***.***> wrote:
Okay let's leave as-is then!
On Fri, Jan 27, 2017 at 4:52 PM Kevin Ushey ***@***.***>
wrote:
> We could, but note that those methods are indeed exported + visible on
> the search path (ie, they're used both in S3 dispatch and are exported as
> though they are free functions). We also show those completions in the
> RStudio IDE and nobody has complained thus far.
>
> —
> You are receiving this because you modified the open/close state.
>
>
> Reply to this email directly, view it on GitHub
> <#9 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAGXx13vTJdYuwv_6EZE7Z39u-KUMAcNks5rWmcugaJpZM4LuMsF>
> .
>
|
NOTE: not yet ready for merge but opening a PR for discussion.
This PR will be used to add autocompletion support to the editors used in an R Markdown tutorial document. Right now we just have the main skeleton necessary for registering a custom completer on Ace; hooking that up to R with a basic completion system is still a WIP.
Now that we're planning different tutor modules (e.g. autocompletion, diagnostics), should we consider generating a single minified
tutor.min.js
from the coretutor.js
stuff alongside thetutor-autocompletion.js
file + (pending) diagnostics?