-
Notifications
You must be signed in to change notification settings - Fork 148
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
Support JupyterLab 2.2 #301
Conversation
Note: disable pre-commit hook for now as it was destroying code and could not work around it. @bollwyvl I am prettier illiterate; it is breaking |
You can disable it explicitly for a single file in Anyhow, can you post an example of before/after what it's doing as a gist? Maybe we need to upgrade? Please, let's not downgrade, as that would ruin my world on #278, which I'm already scared of fixing the bitrot on. But indeed, who lints the linters! I disable the hooks, too, because I don't like node doing anything i don't type out with |
packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts
Outdated
Show resolved
Hide resolved
version "7.8.3" | ||
resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.8.3.tgz#33e25903d7481181534e12ec0a25f16b6fcf419e" | ||
integrity sha512-a9gxpmdXtZEInkCSHUJDLHZVBgb1QS0jhss4cPP93EW7s+uC5bikET2twEF3KV+7rDblJcmNvTR7VJejqd2C2g== | ||
"@babel/code-frame@^7.0.0", "@babel/code-frame@^7.10.4": |
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.
it may well be time to do a full re-solve of all our deps. We should probably do some better sanity pins on, for example, the node-based language servers as well.
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.
surprisingly, seems to be largely working :) the most important trick was in krassowski@bf52857
Also, why didn't Azure CI fire? |
Well, seems that running |
All completer tests now pass locally! CI (https://dev.azure.com/krassowskimichal/jupyterlab-lsp/_build) has issues with backend lint:
which seems like a good mypy catch because
so we should go for |
Yeah, |
Hooray, azure kicked in after merging master this time! |
I've done some digging on that error code before... Sometimes it's enough
to ensure the thing being selected by this keyword is visible, _and_
unique... Will dig into it this weekend, might have to rebuild my windows
VM.
|
Now the question is why
fail while MacOSXThreeSix, WindowsThreeSix, MacOSXThreeEight and all Linux tests pass. I see no pattern... |
Well synced, your message appeared 0.5 second before I pressed enter ;) |
Also, the win 36 is kind of "best effort" because of ignoring some previous
pyls issues... So it's not really an indicator for a lot of features. Will
revisit...
…On Fri, Aug 7, 2020, 09:37 Michał Krassowski ***@***.***> wrote:
Well synced, your message appeared 0.5 second before I pressed enter ;)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/krassowski/jupyterlab-lsp/pull/301#issuecomment-670520871>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAALCRHU25QGXR5JWELG763R7P7TPANCNFSM4PLCQU2Q>
.
|
Now archspec (whatever this is) also fails to conda install on Windows...
|
On windows: cannot reproduce the archspec thing (not familiar with it), but can consistently reproduce the |
@@ -108,6 +130,7 @@ Cell Editor Should Equal | |||
Select Completer Suggestion | |||
[Arguments] ${text} | |||
${suggestion} = Set Variable css:.jp-Completer-item[data-value="${text}"] | |||
Wait Until Element Is Visible ${suggestion} timeout=10s |
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.
Hopefully this works! 🤞
@@ -1,7 +1,7 @@ | |||
parameters: | |||
name: Linux | |||
packages: '' | |||
install_cmd: conda install -yn base -c conda-forge conda python-libarchive-c | |||
install_cmd: conda install -yn base conda |
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.
I guess this is an attempt to tackle the archspec
issue?
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.
yes, the thinking is get something as vanilla as possible, especially for windows, while we get this sorted. who knows what state it's in when they cook the docker images? We could also pin harder, e.g. 4.8
or something.
if we want to start getting far off vanilla, we could investigate installing mamba
in this step instead of upgrading conda
... no doubt it could shave off some minutes, at the price of sometimes giving different solves. definitely a separate PR, though!
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.
i also need to investigate caching (or packaging) the tectonic stuff.
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.
Was just thinking about mamba while reading this
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.
I'll open an issue...
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.
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.
LGTM
References
Supersedes #239, fixes #241.
Code changes
ICompletionItem
interfaceICompletionItemsConnector
over simpleDataConnector
User-facing changes
In addition to the new shiny completer widget (see all three PRs for jupyterlab/jupyterlab#7983):
.
once does not cause autocompletion to insert text if only one item was found. Pressing tab twice does the trick.label
rather thaninsertText
displayedBackwards-incompatible changes
ICompletionItemsConnector
is a breaking change and not backwards compatible with JupyterLab 2.1Chores