Unhelpful behavior with "Only in Selection" search and no selection

SabreCat commented 5 years ago

Prerequisites

[ ] Put an X between the brackets on this line if you have done all of the following:
- Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
- Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
- Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
- Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
- Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

If you have the "Only in Selection" toggle enabled, then search the current buffer without any selection currently highlighted, the search automatically highlights the first result. This means that attempts to proceed to the next search match within the document fail, as the search then collapses to encompass only that first hit. It ends up being a nuisance behavior, as it never feels expected and requires that the user then toggle the search mode and start over.

I frequently run into this as I tend to alternate between in-selection and full-document searches and search-replace ops.

Steps to Reproduce

Toggle "Only in Selection" on
Without selecting anything, search for a string found multiple times in your open file
Attempt to press Enter in the search field to go from one search to the next

Expected behavior:

Search moves from one hit to the next, as if you were in selection-agnostic search mode

Actual behavior:

The search finds the first hit, then stops and loops over that result repeatedly

Reproduces how often:

100%

Versions

Atom : 1.40.1 Electron: 3.1.10 Chrome : 66.0.3359.181 Node : 10.2.0

Windows 10 Home version 1809

Additional Information

Some possible solutions or mitigating changes:

If the user is in selection-only mode, but has nothing selected, select the whole document before beginning the search.
If the user is in selection-only mode, but has nothing selected, never find anything on a search (annoying, but less annoying than the current behavior)
If the user is in selection-only mode, but has nothing selected, don't automatically select search results when you scroll to them
Provide a keyboard shortcut to quickly toggle "Only in Selection" off when you discover you don't want it

rsese commented 5 years ago

4. Provide a keyboard shortcut to quickly toggle "Only in Selection" off when you discover you don't want it

You can add this to your keymap:

'.platform-win32 .find-and-replace':
  'ctrl-alt-s': 'find-and-replace:toggle-selection-option'

Then in the situation you describe where pressing Enter doesn't move past the first result, you can use this (or whatever keybinding you want) to disable Only in Selection.

AzzaAzza69 commented 3 years ago

I would suggest on display of find/replace panel, if the user has something selected then it defaults the "Selection only" on. The user can then unselect text in the editor or unselect the "Selection only" as desired.

ferdnyc commented 3 years ago

If you have the "Only in Selection" toggle enabled, then search the current buffer without any selection currently highlighted, the search automatically highlights the first result. This means that attempts to proceed to the next search match within the document fail, as the search then collapses to encompass only that first hit. It ends up being a nuisance behavior, as it never feels expected and requires that the user then toggle the search mode and start over.

It seems to be a bit worse than that, to me. Here's what I'm seeing, at least in Atom 1.56.0 on Linux x86_64 — behavior confirmed even with atom --safe.

Even with a selection "Find" will collapse that selection to the first match when you search. Meaning you can only ever find the first hit in your selection, and as a bonus you also lose the selection.
"Find All" will degrade a bit more slowly, but still breaks:
1. When you first activate it, it highlights all of the matches and leaves the cursor on the last one, showing e.g. "4 of 4" for the match position.
2. Hitting either "Find All" or "Find" from that point moves to the first match, and changes the position to "4 found".
3. At this point, hitting "Find" a second time leaves you on that same first match, but now shows "1 found".
4. Hitting "Find All" a second or more times, OTOH, also leaves you stuck on the first match, the only difference being that the position stays at "4 found" and all matches remain highlighted.

Inspecting the DOM with the dev console, it seems that Atom does draw a distinction between .selections and .find-results, two distinct type of .highlight <div>s created as children of a <div class="highlights"> container.

<div class="highlights style="style="contain: strict; position: absolute;
 overflow: hidden; user-select: none; height: 48279px; width: 920px;">

  <!-- A mouse selection creates one or more (depending on the shape) of these: -->
  <div class="highlight selection">...</div>

  <!-- "Find All" creates, for N matches, N-1 of these: -->
  <div class="highlight find-result">...</div>
  <!-- And exactly one of these: -->
  <div class="highlight current-result">...</div>

  <!-- ...But it **also** replaces the previous selection with N of these: -->
  <div class="highlight selection">...</div>

  <!-- Whereas "Find" simply replaces the previous selection with
         a single one of these, which follows the current-result: -->
  <div class="highlight selection">...</div>

Because activating any "Find" command for some reason blows away the user's text selection in favor of selecting some or all of the results (even though those selections are redundant with the current-result and/or find-result highlights it also creates), having "Find in selection" enabled becomes instantly useless.

ferdnyc commented 3 years ago

I would suggest on display of find/replace panel, if the user has something selected then it defaults the "Selection only" on. The user can then unselect text in the editor or unselect the "Selection only" as desired.

I would flip that around slightly, and make "Find in selection" default to the entire buffer, if nothing is selected. That way, the toggle for "Find in selection" never has to auto-enable, it can be persisted across uses of the find panel or even entire Atom sessions, and it effectively becomes a preference switch to activate/deactivate the search behavior you describe, just without the magical auto-enabling.

Reversing it like that means that people who never want "Find in selection" can just leave it toggled off, it'll stay toggled off, and nobody is ever forced to unselect text or toggle switches to scope search the way they want it.

ferdnyc commented 3 years ago

@AzzaAzza69

I would suggest on display of find/replace panel, if the user has something selected then it defaults the "Selection only" on. The user can then unselect text in the editor or unselect the "Selection only" as desired.

Heh! I discovered there was an old plugin to do exactly that (atom-auto-replace-in-selection). Turns out, getting the logic right on that can be trickier than it seems.

(It's a common pattern to select something, copy, then pull up Find: and paste into it. A naive auto-find-in-selection implementation can break that pattern.)

atom / find-and-replace