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

Find text only searches part of file #6697

Closed
PAGWatson opened this issue Jan 16, 2023 · 11 comments · Fixed by #6905
Closed

Find text only searches part of file #6697

PAGWatson opened this issue Jan 16, 2023 · 11 comments · Fixed by #6905
Labels
bug tag:Release Blocker A must-have bug for the milestone to which it is tagged
Milestone

Comments

@PAGWatson
Copy link

If I open a long .py file in Notebook 7.0.0a10 and try to search for text with Ctrl+F, text only seems to be found if it appears within the portion of the file being viewed on screen, and it says there are zero instances otherwise. If I try it in jupyterlab (4.0.0a32), the text is found as long as it exists somewhere in the file, so this seems to be an issue specific to Notebook. It would be desirable to be able to search the whole of python scripts, text files etc.

@PAGWatson PAGWatson added the bug label Jan 16, 2023
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to issues that need triage label Jan 16, 2023
@jtpio jtpio added this to the 7.0 milestone Jan 16, 2023
@jtpio
Copy link
Member

jtpio commented Jan 16, 2023

Thanks @PAGWatson for opening this issue 👍

This is indeed likely an issue in Notebook 7 if it works with the latest JupyterLab 4.0 pre-release. Maybe this was introduced in #6539.

Would you be able to provide a screenshot to make it easier to reproduce the issue?

@jtpio jtpio removed the status:Needs Triage Applied to issues that need triage label Jan 16, 2023
@PAGWatson
Copy link
Author

Sure. As an example, one of my scripts has a function "get_color_cycle", and if I search for this in the searchbar brought up by Ctrl+F when viewing the top of the file, then it returns zero results:

image

If I scroll down to show the function in the browser window and search again, the text is found:

image

My guess was that the notebook is only loading the file contents near the location that is being viewed or something? For small text files and scripts, I'd guess it's generally better to load the whole thing.

@jtpio
Copy link
Member

jtpio commented Jan 17, 2023

Ah interesting thanks for sharing the screenshots.

If I try it in jupyterlab (4.0.0a32), the text is found as long as it exists somewhere in the file

In JupyterLab, does the Ctrl-F trigger the browser search popup like in Notebook 7? Or the JupyterLab search / replace tool?

@Higgs32584
Copy link

Higgs32584 commented Mar 3, 2023

So I looked over this bug to see if I could replicate it. I replicated it, but the question is not so much a bug as it is functionality.

So the HTML search bar does not pick up on the text in the python file, but the localized, built-in jupyter notebook search bar does. I guess it is a matter of if we want the HTML search bar to be able to search the python code or not.

case#3

on built into the application, where
screenshot#2

@fcollonval
Copy link
Collaborator

Small comment: the browser search will always fail as the text editor (i.e. codemirror 6) is only rendering the text content that fits in the viewport.

There are two solutions:

  • most desirable: use the jupyter search (as implemented in JupyterLab)
  • less desirable: use the codemirror 6 search extension

@jtpio
Copy link
Member

jtpio commented May 31, 2023

This is still an issue in the latest main, but using the built-in Find should work:

text-editor-search.mp4

The Ctrl-F keybinding was disabled by default on purpose in jupyterlab/retrolab#294, because searching on a page this way is so common.

But now that the rendering of the notebook has changed in JupyterLab (and in Notebook 7), maybe Ctrl-F should trigger the JupyterLab search / replace popup instead of the browser one to make sure we can find the content on the whole page and in the editors? This should be matter of removing the override here (or the whole file):

{
"command": "documentsearch:start",
"keys": ["Accel F"],
"selector": ".jp-mod-searchable",
"disabled": true
}

@jtpio
Copy link
Member

jtpio commented Jun 7, 2023

maybe Ctrl-F should trigger the JupyterLab search / replace popup instead of the browser one to make sure we can find the content on the whole page and in the editors? This should be matter of removing the override here (or the whole file):

In that case the new default would be to open the JupyterLab search box when hitting Ctrl-F. But we could also document how to disabled the keybinding to Ctrl-F would use the browser search bar like now. Just to make it configurable by end users.

@ericsnekbytes
Copy link
Collaborator

I'm looking at this now and have reproduced the issue and did some basic testing on the proposed solution. I have opened a PR with that change suggested above by @jtpio. I'm also planning to investigate the documentation changes needed for this.

@ericsnekbytes
Copy link
Collaborator

@jtpio How would a user disable the shortcut? In the keyboard shortcuts menu in the settings editor, I see a way to view and add shortcuts, but can they be disabled? I didn't see an option for that with this document search shortcut (was able to see the shortcut for document search but when selecting and deleting the keys, it just goes back to its previous value)...

@jtpio
Copy link
Member

jtpio commented Jun 8, 2023

Right I'm not sure it's possible to disable the shortcut via the Settings Editor UI.

However it should be possible with the Advanced Settings Editor by adding "disabled": true:

disable-shortcut-notebook.mp4

@ericsnekbytes
Copy link
Collaborator

@jtpio Thanks, I'm gonna work up documentation for this to include in the PR.

@jtpio jtpio linked a pull request Jun 9, 2023 that will close this issue
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug tag:Release Blocker A must-have bug for the milestone to which it is tagged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants