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

Fixes "replace selection" behavior when nothing is selected #251

Merged
merged 4 commits into from
Jul 5, 2023

Conversation

JasonWeill
Copy link
Collaborator

Fixes #239. To repro:

  1. In cell 1, highlight some text, and use the Jupyter AI's chat UI to ask a question, with both the "include selection" and "replace selection" boxes checked.
  2. After the prompt runs and the output replaces the selection, run cell 1. Cell 2 is now active, and nothing in cell 2 is selected.
  3. Ask a question in the chat UI — note that neither "Include selection" nor "replace selection" appear as options.
  4. Verify that the response is not added to either cell 1 or cell 2.

@JasonWeill JasonWeill added bug Something isn't working @jupyter-ai/chatui labels Jun 30, 2023
dlqqq
dlqqq previously requested changes Jul 5, 2023
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.

There is a more robust solution to this that allows us to fix the bug with minimal changes. The root cause of the issue is that the getTextSelection() function, defined in src/selection-watcher.ts, returns a Selection object even when no text is selected.

In src/selection-watcher.ts, modify the implementation of getTextSelection() to

  ...

  const text = editor.model.sharedModel
    .getSource()
    .substring(startOffset, endOffset);
  
  // add this block
  if (!text) {
    return null;
  }

  ...

and revert the other changes to src/components/chat.tsx. I've tested this locally, though you should verify this change independently as well.

@JasonWeill
Copy link
Collaborator Author

@dlqqq Tested locally, successfully. Thanks for your help!

Removes unneeded comment
@JasonWeill JasonWeill dismissed dlqqq’s stale review July 5, 2023 22:26

Changes incorporated per requester

@dlqqq dlqqq merged commit b62c816 into jupyterlab:main Jul 5, 2023
3 checks passed
dbelgrod pushed a commit to dbelgrod/jupyter-ai that referenced this pull request Jun 10, 2024
…ab#251)

* Check that selection has text before replacing

* Disable replaceSelection if nothing is selected

* Reverts changes to chat.tsx, modifies selection-watcher per @dlqqq

* Update chat.tsx

Removes unneeded comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working @jupyter-ai/chatui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Replace selection" option retains its prior selection, even if the checkbox is no longer visible
2 participants