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

[Jim Porter]

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.)