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

Add new InputModel class for managing input state #171

Merged
merged 10 commits into from
Mar 3, 2025

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Feb 17, 2025

This PR adds an InputModel class to handle input state.
This is proposed for:

  • simplifying the use of the chats commands, that needs to get the input content, the current word and to be able to replace it.
  • enhancing the messages editiing, by allowing adding and deleting attachments.

By default, the chat model includes an input model, for the "main" input of the chat panel. When editing a message, a new input model is created for this edition, and deleted when edition is over (cancelled or validated).

Fixes #163

Copy link
Contributor

Binder 👈 Launch a Binder on branch brichet/jupyter-chat/input-model

@brichet brichet added the enhancement New feature or request label Feb 17, 2025
@brichet brichet marked this pull request as ready for review February 17, 2025 16:25
@dlqqq
Copy link
Member

dlqqq commented Feb 17, 2025

@brichet I think this PR will cause some conflicts with the chat commands PR in #161. I think it would be best to rebase this branch & fix conflicts after merging #161. I'll give this a review after.

@dlqqq dlqqq changed the title Add an input model Add new InputModel class for managing input state Feb 17, 2025
@dlqqq
Copy link
Member

dlqqq commented Feb 17, 2025

#148 should probably also be merged first to address any merge conflicts. I've approved that PR.

@brichet brichet marked this pull request as draft February 17, 2025 22:26
@brichet brichet marked this pull request as ready for review February 18, 2025 13:59
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brichet Wow, thanks for working on this so quickly! All of the high-level changes look good. I left some feedback for you below, but still need more time to complete a full review.

My day tomorrow will be full of meetings, so I will likely need until the end of Thursday to finish reviewing this. Feel free to work on another issue using the Kanban board: https://github.com/orgs/jupyterlab/projects/9

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brichet This looks perfect, thank you for building this!

@brichet brichet merged commit b1993dc into jupyterlab:main Mar 3, 2025
12 checks passed
@brichet brichet deleted the input-model branch March 3, 2025 16:52
@brichet
Copy link
Collaborator Author

brichet commented Mar 3, 2025

Thanks for the review @dlqqq

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subissue: Lift chat input state to ChatModel
2 participants