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

File completer #5049

Merged
merged 10 commits into from Aug 10, 2018
Merged

File completer #5049

merged 10 commits into from Aug 10, 2018

Conversation

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Aug 5, 2018

Adds completer and tooltip support to text editors. Fixes #3483.

It is only enabled when there is an attached console to the text editor. @afshin Do you have any thoughts on how the performance of this might suffer for larger text files?

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 5, 2018

My 2 cents is that we should wait for Monaco+LSP for this. I don't like the UX of having to start a console to get tab completion.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 5, 2018

This came up at the Jupyter Open Studio yesterday at Bloomberg, where multiple people asked after the feature.

It's true that it's a bit awkward to have to hook up a console, so I have no particular objection to waiting for LSP.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 5, 2018

For those following at home: jupyterlab/jupyterlab-monaco#12

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 5, 2018

This came up at the Jupyter Open Studio yesterday at Bloomberg, where multiple people asked after the feature.

It's true that it's a bit awkward to have to hook up a console, so I have no particular objection to waiting for LSP.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Aug 6, 2018

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 6, 2018

Tokenizing sounds good for now.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 6, 2018

We could tokenize the file when there is no kernel associated with it, and use the completer when there is. This would have the side effect of fixing #4876.

@ccordoba12
Copy link

@ccordoba12 ccordoba12 commented Aug 6, 2018

I think Codemirror has some form of token-based completions. At least that's the case in the classic notebook, where tokens are shown in blue in its completion widget and below the completions obtained from the kernel.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 9, 2018

@ian-r-rose, this is looking great so far. Do you plan to add the ability to connect to the kernel after the fact (and then handle when it goes away)?

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 9, 2018

Yeah, I am currently reworking things so as not to rely on the console tracker. I was finding that the lifecycle logic for interacting with console widgets was getting top complex, so I am trying to get it working by interacting directly with the SessionManager, and keeping the completer up-to-date with that.

A nice consequence of this is that there just needs to be a kernel with the associated path running, and you can safely close the console.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 9, 2018

nb. This is similar to how the filebrowser handles the "running" status indicator in the file listing. So if a text file has a green dot next to it, then it should also get kernel completions. If it does not, it only gets context completions.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 9, 2018

if a text file has a green dot next to it, then it should also get kernel completions. If it does not, it only gets console completions.

I like that consistency!

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 9, 2018

Fixes #4876.

Okay, this got away from me a bit in terms of complexity, but I think it's ready for a look. It does the following:

  1. Adds getTokenForPosition and getTokens to the abstract editor interface, allowing the editor users to tokenize their contents. Both monaco and codemirror provide functionality for this. getTokens is potentially an expensive operation, so tokenizing the editor contents might be slow for a large file. On the other hand, it is a local operation, not having to make the network round-trip. I'm not sure when the crossover is for the kernel response vs the tokenization.
  2. Implements getTokenForPosition and getTokens in the codemirror plugin.
  3. Adds a new ContextConnector, implementing IDataConnector, and providing context hints for the completion handler for a given editor. Also adds a hybrid connector that merges the results of the ContextConnector completions and the KernelConnector completions.
  4. Consoles and notebooks use the hybrid completer, giving both kernel and local token completions.
  5. Files use the ContextCompleter normally. However, if there is a kernel running with the same path as the file, it also gets kernel completions.
  6. Files can also get tooltip information if there is a kernel running with their path.

Also, this was not nearly so painful as it could have been: @afshin did a really nice job architecting the completer machinery.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 9, 2018

One consequence of this that merits some discussion: this affects the ability to insert tabs into text files when they are not at the beginning of the line. This is the same behavior that notebook cells have.

The completer is invoked by a tTab press when the jp-mod-completer-enabled CSS class is applied to the editor. Perhaps we should not apply that class for a small set of non-code mimetypes like text/plain, text/markdown, or with extension .tsv.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 9, 2018

Maybe we should disable the completer at the start of a line in general? Seems odd to invoke completion with really no context.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 9, 2018

Sorry, I was unclear: The user can still insert tabs at the start of a line (or if there is only whitespace behind it). It is when the user's cursor is within a token that it affects the ability to insert tabs.

The old behavior can still be obtained by removing the keyboard shortcut for invoking the completer in file editors.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 10, 2018

This is beautiful, @ian-r-rose, bravo!

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 10, 2018

I think the current behavior is fine and consistent. We can revisit if it causes a problem.

@blink1073 blink1073 merged commit ba93c9b into jupyterlab:master Aug 10, 2018
2 checks passed
@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 10, 2018

Yep, it's fairly self-contained, so if Monaco and LSP land, and we want to remove this, it will be an easy task.

@sntgluca
Copy link

@sntgluca sntgluca commented Dec 27, 2018

Dear @ian-r-rose, I find this a tremendous move forward!

What do you think about generalising this idea and allowing to connect to existing kernels, rather than opening a console?
IMHO it would make sense, imagine you've a kernel open from a notebook and want to update some libraries in a related file.
Would it be difficult?

Thanks!

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Jan 7, 2019

@sntgluca I think it is reasonable to connect this to an existing kernel. In fact, I believe if you create a console for a text file you should be able to select an existing kernel from the dropdown. Afterwards you can safely close the console, but the connection will still exist. That interaction should work, but is a bit awkward if you don't want the console, so if people have ideas for a better UI and want to work on that, I am all ears!

@krinsman
Copy link
Contributor

@krinsman krinsman commented Aug 1, 2019

Is there a central thread somewhere tracking the status of all of these projects? (I'm familiar with the Monaco JpL extension, don't know what LSP stands for.)

It would be nice if JupyterLab had VSCode style auto-completion (i.e. it happens automatically, no tab keys), or if such a feature could be enabled in preferences/settings even if not by default.

The Monaco JupyterLab extension appears to be abandoned, so I'm a little concerned this will never happen and that it will always be necessary to use another tool with JupyterLab when coding.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 1, 2019

LSP is the Language Server Protocol, which is how VSCode gets their intellisense. See #2163 for an issue about it.

@krinsman
Copy link
Contributor

@krinsman krinsman commented Aug 2, 2019

@jasongrout Thank you for the clarification!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants