bug#71576: 30.0.50; [PATCH] Improve performance of Comint/Eshell password prompt handling

[Jim Porter <jporterbugs@gmail.com>]

On 6/16/2024 3:53 AM, Stefan Kangas wrote:
> Thanks.  I'd be okay with putting this patch in Emacs 30, but let's see
> what other people think.

Sounds good to me.

>> +(defcustom comint-password-prompt-max-length 256
>> +  "The maximum amount of text to examine when matching password prompts.
>> +When non-nil, only examine the last N characters of a block of output.
>> +If nil, examine all the output.
>> +
>> +This is used by `comint-watch-for-password-prompt' to reduce the amount
>> +of time spent searching for password prompts."
>> +  :version "30.1"
>> +  :type '(choice natnum
>> +                 (const :tag "Examine all output" nil))
>> +  :group 'comint)
> 
> If this is hardcoded in Tramp, are we sure that we need this as an
> option?  I'd suggest making it into a defconst or defvar instead.

I don't have a strong opinion here, so I'll wait to see if a majority 
forms around this...

>> +      (goto-char (if eshell-password-prompt-max-length
>> +                     (max eshell-last-output-block-begin
>> +                          (- eshell-last-output-end
>> +                             eshell-password-prompt-max-length))
>> +                   eshell-last-output-block-begin))
>> +      (when (let ((case-fold-search t))
>> +              (re-search-forward eshell-password-prompt-regexp
>> +                                 eshell-last-output-end t))
> 
> Could this be simplified using re-search-backward with the BOUND
> argument instead?

I tried, but since I think most of this logic is necessary, it just 
amounted to swapping the 'if' block with 'eshell-last-output-end'. 
Performance doesn't look any different, and I find the current way a bit 
more readable.

(This could probably be simplified if we want to require that 
'eshell-password-prompt-max-length' be non-nil though.)