bug#47599: [External] : bug#47599: 28.0.50; Feature request improve/update isearch

[Juri Linkov <juri@linkov.net>]

>>>> While we're improving isearch, there is another minor change that
>>>> I think would improve its behavior, namely to go to the next/previous
>>>> match when changing the search direction, instead of hitting the same
>>>> match before moving to the next/previous one with the next
>>>> C-s/C-r. This change is so small that I'm not sure it's worth creating
>>>> a defcustom for it, but you might have a different opinion.
>>> Please do NOT do this.  There are reasons we want to stay at the same
>>> search hit.
>>
>> Okay, so I have the answer to my own question: this should become yet
>> another user option.
>
> And here is the corresponding patch.

Thanks, finally there is an option to avoid typing extra C-r.

> +(defcustom isearch-direction-change-changes-match nil
> +  "Whether a direction change should move to another match.
> +When `nil', the default, a direction change moves point to the other
> +end of the current search match.
> +When `t', a direction change moves to another search match, if there
> +is one."
> +  :type '(choice (const :tag "Remain on the same match" nil)
> +                 (const :tag "Move to another match" t))

Is it possible to find a clearer name?
Maybe isearch-repeat-on-direction-change would be better
with the prefix 'isearch-repeat-' to hint that it applies
to the commands 'isearch-repeat-*'?

>      ;; C-s in reverse or C-r in forward, change direction.
> +    (if (and isearch-other-end isearch-direction-change-changes-match)
> +        (goto-char isearch-other-end))

This breaks the following feature:

When isearch-forward is t:
- C-1 C-r moves to the previous match (like your patch does without 'C-1')
- C-2 C-r moves to the second previous match
- C-u -1 C-r moves to the next match
- C-u -2 C-r moves to the second next match

This is due to these lines in isearch-repeat-backward:

               ;; Reverse the direction back
               (isearch-repeat 'backward))
              (t
               ;; Take into account one iteration to reverse direction
               (when isearch-forward (setq count (1+ count)))

When the new option is non-nil, there is no need to increment 'count'.
Also the new option should be let-bound to nil around the call to
'(isearch-repeat 'backward)' above to just change the direction back
without moving to the next match.

The same applies to isearch-repeat-forward and when isearch-forward is nil.