GNU bug report logs - #71576
30.0.50; [PATCH] Improve performance of Comint/Eshell password prompt handling

Previous Next

Package: emacs;

Reported by: Jim Porter <jporterbugs <at> gmail.com>

Date: Sat, 15 Jun 2024 20:00:03 UTC

Severity: normal

Tags: patch

Found in version 30.0.50

Done: Jim Porter <jporterbugs <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>, 71576 <at> debbugs.gnu.org
Subject: bug#71576: 30.0.50; [PATCH] Improve performance of Comint/Eshell password prompt handling
Date: Sun, 16 Jun 2024 18:18:01 -0700
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.)




This bug report was last modified 1 year and 26 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.