-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add error field and animation in inline completion #15344
Add error field and animation in inline completion #15344
Conversation
Thanks for making a pull request to jupyterlab! |
Thanks for submitting your first pull request! You are awesome! 🤗 |
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.
Thank you for starting this @Wzixiao. Please see a few suggestions/questions below.
@@ -127,6 +127,7 @@ export class ProviderReconciliator implements IProviderReconciliator { | |||
// Stream an update. | |||
item.insertText = updated.insertText; | |||
item.lastStreamed = addition; | |||
item.isError = reply.response.isError; |
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.
Does it also need to be set in fetchInline
?
@Wzixiao are you interested in finishing this PR? Our merge window for 4.1 will close soon. |
In future this could include additional fields like `type` or `traceback`
How to test:
|
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.
This is good to go from my side - thank you @Wzixiao for starting this! I will wait a while before merging in case if anyone else wants to review the latest version (but preferably further enhancements would be tracked in separate issues and addressed in a follow-up PRs).
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.
thanks @Wzixiao and @krassowski
References
Fixes #15343
Code changes
packages/completer/src/ghost.ts
packages/completer/src/inline.ts
packages/completer/src/reconciliator.ts
packages/completer/src/tokens.ts
packages/completer/style/base.css
User-facing changes
deom_error_animation.mp4