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

Notebook: find doesn't search in output #94239

Closed
deepak1556 opened this issue Apr 2, 2020 · 7 comments
Closed

Notebook: find doesn't search in output #94239

deepak1556 opened this issue Apr 2, 2020 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality notebook-output
Milestone

Comments

@deepak1556
Copy link
Collaborator

Testing #93742

  • Create a notebook with query label:electron $repo sort desc by reactions
  • Search for some text that is also in the cell output
  • Notice results are only highlighted for text in query cell
@jrieken jrieken removed their assignment Apr 2, 2020
@rebornix rebornix added the feature-request Request for new features or functionality label Apr 2, 2020
@rebornix rebornix added this to the Backlog milestone Apr 2, 2020
@rebornix
Copy link
Member

rebornix commented Apr 2, 2020

Putting it in backlog first considering its complexity.

@rebornix
Copy link
Member

I should explain its "complexity" in details. It's not totally impossible to implement search in the webview but the catch is how many bugs or limitations it has.

Firstly, to archive the best result, we want to let the browser to do the search other than us traverse the tree as we might not able to handle cases like <strong>W</strong>arn. Most modern browsers implement window.find https://developer.mozilla.org/en-US/docs/Web/API/Window/find but it's not standard and the browser behaviors vary.

window.find(aString, aCaseSensitive, aBackwards, aWrapAround,
            aWholeWord, aSearchInFrames, aShowDialog);

Also not every browser implements all flags, e.g., Firefox doesn't support aWholeWord. As shown in the function signature, it doesn't support regex. We can implement a simple text (with case sensitivity support) like

const resultRanges = [];
if(window.find) {
  while(window.find(query)) {
    const range = window.getSelection().getRangeAt(0);
    resultRanges.push(range);
  }
}

This algorithm might be good for a lot of cases but not sure how much it can cover (90%, or 50%? It depends on output renderers). The drawback of this approach is the bugs browsers currently have for window.find, and also some attempt to kill this API completely:

IMHO it's a good API to start with but we might need a champion/ambassador for it to convince the browsers that it's a good API to have for web apps.

@rebornix
Copy link
Member

Another challenge we have is we are only rendering outputs when it become visible. It means you can't search for things which are not rendered once yet (cell input is good though), even if you know they exist.

@nicocanali
Copy link

Is there any update on this issue (since it's still open)? 😄 🙏

@rebornix
Copy link
Member

rebornix commented Dec 6, 2022

We now support searching cell outputs.

@rebornix rebornix closed this as completed Dec 6, 2022
@nicocanali
Copy link

@rebornix thanks for the update!

Do I need to do anything special to enable this? For now, nothing seems to have changed (Insiders version as well) 🤔

@RyanBlace
Copy link

vscode_jupyter_search

@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality notebook-output
Projects
None yet
Development

No branches or pull requests

6 participants