bug#73768: 'read-passwd-mode' can clobber user defined mode lines

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

After reading a password here I sometimes end up with seeing one or two
eye icons in mode lines of buffers.  Other buffers may get a blank mode
line.  I suppose the culprit is

     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
       (when (listp mode-line-format)
         (setq mode-line-format (cdr mode-line-format)))))

which can do something arbitrary to user defined mode lines which I use.
Eventually, I have to kill or revert all involved buffers every time
I've read a password.

At the very least the code should try to identify the item it added
earlier, remove the car only if it matches that item and restore the
previous mode line format in case the items don't match.  Also it should
check whether 'read-passwd--mode-line-buffer' still exists.

In either case, it would be nice to make showing the eye icon
customizable by the user.  I found no way to do that.

Thanks, martin

In GNU Emacs 31.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version
  3.24.38, cairo version 1.16.0) of 2024-10-12 built on restno
Repository revision: 6e80509099e27e237787186f4e8731bfdfb9b00d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
  'configure --with-gif=ifavailable --with-tiff=ifavailable
  --with-gnutls=no --without-pop 'CFLAGS=-O3 -g3 -no-pie''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GSETTINGS HARFBUZZ JPEG LIBSELINUX MODULES
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: de_AT.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: ELisp/d

Minor modes in effect:
   pop-up-mini-mode: t
   scroll-restore-mode: t
   regexp-lock-mode: t
   eldoc-tooltip-mode: t
   tooltip-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   context-menu-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   minibuffer-regexp-mode: t
   column-number-mode: t
   line-number-mode: t
   auto-fill-function: my-lisp-mode-auto-fill
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   temp-buffer-resize-mode: t

Load-path shadows:
None found.

Features:
(shadow mail-extr warnings emacsbug message mailcap yank-media puny
rfc822 mml mml-sec password-cache epa epg rfc6068 epg-config gnus-util
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs vc-git files-x
vc-dispatcher elp vc-hg diff-mode track-changes ediff-vers ediff
ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util local-tags goto-last-change speck pop-up-mini info-look
find-func srt-mode derived elinfo-support elinfo texinfo
texinfo-loaddefs info shell pcomplete comint subr-x ansi-osc ansi-color
sidebar bookmark text-property-search sort m&d scroll-restore
regexp-lock time-stamp cl-macs eldoc-tooltip pcase easy-mmode etags
fileloop generator xref project byte-opt bytecomp byte-compile ring
find-dired dired dired-loaddefs cus-edit pp cus-load icons wid-edit
cl-loaddefs cl-lib jka-compr ls-lisp gv rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd touch-screen
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
gtk x-toolkit xinput2 x multi-tty move-toolbar make-network-process
emacs)

Memory information:
((conses 16 271421 29959) (symbols 48 16095 0) (strings 32 44610 3502)
  (string-bytes 1 1330220) (vectors 16 24496)
  (vector-slots 8 277397 25280) (floats 8 109 1037)
  (intervals 56 9458 42) (buffers 984 34))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Martin,

> After reading a password here I sometimes end up with seeing one or two
> eye icons in mode lines of buffers.  Other buffers may get a blank mode
> line.  I suppose the culprit is
>
>     (with-current-buffer read-passwd--mode-line-buffer
>       ;; Remove `read-passwd--mode-line-icon'.
>       (when (listp mode-line-format)
>         (setq mode-line-format (cdr mode-line-format)))))
>
> which can do something arbitrary to user defined mode lines which I use.
> Eventually, I have to kill or revert all involved buffers every time
> I've read a password.
>
> At the very least the code should try to identify the item it added
> earlier, remove the car only if it matches that item and restore the
> previous mode line format in case the items don't match.  Also it should
> check whether 'read-passwd--mode-line-buffer' still exists.

Could you please provide a short recipe I could play with?

> In either case, it would be nice to make showing the eye icon
> customizable by the user.  I found no way to do that.

I'll see what's possible. FTR, non-graphical alternatives are already
defined in read-passwd--show-password-icon and
read-passwd--hide-password-icon.

Could you pls give an example what you would like as replacement?

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Michael

 > Could you please provide a short recipe I could play with?

No, otherwise I would have attached it to my initial post.  It happens
when I run ssh-add in a shell buffer.  But it's not 100% reproducible
and for most of my buffers (car mode-line-format) is invariably "%e".
But I use a separate minibuffer frame so

(window-in-direction 'above (minibuffer-window))

should reliably return nil and (minibuffer-selected-window) would be
used.  'eldoc-mode' is nil so I doubt that it could interfere with its
own ideas about modifying the mode line when I'm in the minibuffer and
possibly prepend the second eye icon I occasionally see.

I'll try to play around with this myself so I can tell you more on what
goes on.

 >> In either case, it would be nice to make showing the eye icon
 >> customizable by the user.  I found no way to do that.
 >
 > I'll see what's possible. FTR, non-graphical alternatives are already
 > defined in read-passwd--show-password-icon and
 > read-passwd--hide-password-icon.
 >
 > Could you pls give an example what you would like as replacement?

Why can't you make that a full-fledged option 'read-passwd-show-icon'
and initialize it to 'graphics-only'?  And why can't you make the icon
part of the prompt with some clickable text or overlay property?  Think
of users who turn mode lines off in some buffers at least.

Thanks, martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]

When in my Emacs with a stand alone minibuffer frame I create a shell
buffer and there do ssh-add, the following happens:

In the lines below 'wid' stands for the window in direction (nil in the
rows below because a stand alone minibuffer frame has no window above),
'msw' the minibuffer selected window, 'glw' the largest window (nil in
the first row below because a stand alone minibuffer frame has no
largest window) and 'sw' the selected window.  'add' and 'remove' stand
for adding and removing the icon. 'before' and 'after' stand for whether
we are before or after doing that.  These are followed by the value of
'read-passwd--mode-line-buffer' and the first two elements of that
buffer's mode line string.

wid .. nil .. msw .. #<window 11 on *shell*> .. glw .. nil .. sw .. #<window 8 on  *Minibuf-1*>
add before .. *shell* .. %e .. mode-line-version
add after .. *shell* .. (:eval read-passwd--mode-line-icon) .. %e
wid .. nil .. msw .. nil .. glw .. #<window 6 on .emacs> .. sw .. #<window 11 on *shell*>
remove before .. .emacs .. %e .. mode-line-version
remove after .. .emacs .. mode-line-version ..

So the window chosen for adding the icon is the value of calling
'minibuffer-selected-window' and everything is OK ('mode-line-version'
is a private variable of mine, ignore it).  The icon's buffer is *shell*
and the icon appears correctly.

But when the icon shall be removed, 'minibuffer-selected-window' returns
nil and the largest window is chosen instead which now happens to show
my init file .emacs.  ‘read-passwd’ leaves the icon in *shell* and kills
the mode line of my .emacs file.

The crucial fact is that the selected window changes from the minibuffer
window to the *shell* window because 'minibuffer-selected-window' is
specified as

   if (minibuf_level > 0
       && MINI_WINDOW_P (XWINDOW (selected_window))
       && WINDOW_LIVE_P (minibuf_selected_window))
     return minibuf_selected_window;

and *shell* is not a mini window.  So 'minibuffer-selected-window'
returns nil in the scenario at hand and the largest window of the
selected window's frame is chosen.  And the selected window obviously
changes because 'read-string' (and in further consequence read_minibuf)
restores the window selected before the minibuffer window was selected.

Concludingly, the current version works when 'window-in-direction'
returns a suitable window and may fail miserably otherwise.

The attached patch fixes the problem here.  But I still think that the
icon should appear in the prompt of 'read-passwd' itself.

martin

[-- Attachment #2: read-passwd.diff --]
[-- Type: text/x-patch, Size: 1430 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..5353a7a55fe 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2534,13 +2534,15 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"
 
-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  ;; Stolen from `eldoc-minibuffer-message'.
+  (unless (and (buffer-live-p read-passwd--mode-line-buffer)
+	       (get-buffer-window read-passwd--mode-line-buffer))
+    (setq read-passwd--mode-line-buffer
+          (window-buffer
+           (or (window-in-direction 'above (minibuffer-window))
+	       (minibuffer-selected-window)
+	       (get-largest-window)))))
 
   (if read-passwd-mode
       (with-current-buffer read-passwd--mode-line-buffer
@@ -2552,7 +2554,8 @@ read-passwd-mode
     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
       (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+        (setq mode-line-format (cdr mode-line-format)))
+      (setq read-passwd--mode-line-buffer nil)))
 
   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

> Hi Michael

Hi Martin,

>>> In either case, it would be nice to make showing the eye icon
>>> customizable by the user.  I found no way to do that.
>>
>> I'll see what's possible. FTR, non-graphical alternatives are already
>> defined in read-passwd--show-password-icon and
>> read-passwd--hide-password-icon.
>>
>> Could you pls give an example what you would like as replacement?
>
> Why can't you make that a full-fledged option 'read-passwd-show-icon'
> and initialize it to 'graphics-only'?

Because Emacs runs also in non-graphical environments.

> And why can't you make the icon
> part of the prompt with some clickable text or overlay property?  Think
> of users who turn mode lines off in some buffers at least.

Minibuffer indications belong to the modeline. And if there is no
modeline, you can always use TAB in the minibuffer to toggle between
password visibility. This is documented in the Elisp manual, (info
"(elisp) Reading a Password") .

Granted, it shall also be mentioned in the Emacs manual, (info "(emacs)
Passwords") . Will do.

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

> When in my Emacs with a stand alone minibuffer frame I create a shell
> buffer and there do ssh-add, the following happens:
>
> In the lines below 'wid' stands for the window in direction (nil in the
> rows below because a stand alone minibuffer frame has no window above),
> 'msw' the minibuffer selected window, 'glw' the largest window (nil in
> the first row below because a stand alone minibuffer frame has no
> largest window) and 'sw' the selected window.  'add' and 'remove' stand
> for adding and removing the icon. 'before' and 'after' stand for whether
> we are before or after doing that.  These are followed by the value of
> 'read-passwd--mode-line-buffer' and the first two elements of that
> buffer's mode line string.
>
> wid .. nil .. msw .. #<window 11 on *shell*> .. glw .. nil .. sw .. #<window 8 on  *Minibuf-1*>
> add before .. *shell* .. %e .. mode-line-version
> add after .. *shell* .. (:eval read-passwd--mode-line-icon) .. %e
> wid .. nil .. msw .. nil .. glw .. #<window 6 on .emacs> .. sw .. #<window 11 on *shell*>
> remove before .. .emacs .. %e .. mode-line-version
> remove after .. .emacs .. mode-line-version ..
>
> So the window chosen for adding the icon is the value of calling
> 'minibuffer-selected-window' and everything is OK ('mode-line-version'
> is a private variable of mine, ignore it).  The icon's buffer is *shell*
> and the icon appears correctly.
>
> But when the icon shall be removed, 'minibuffer-selected-window' returns
> nil and the largest window is chosen instead which now happens to show
> my init file .emacs.  ‘read-passwd’ leaves the icon in *shell* and kills
> the mode line of my .emacs file.
>
> The crucial fact is that the selected window changes from the minibuffer
> window to the *shell* window because 'minibuffer-selected-window' is
> specified as
>
>   if (minibuf_level > 0
>       && MINI_WINDOW_P (XWINDOW (selected_window))
>       && WINDOW_LIVE_P (minibuf_selected_window))
>     return minibuf_selected_window;
>
> and *shell* is not a mini window.  So 'minibuffer-selected-window'
> returns nil in the scenario at hand and the largest window of the
> selected window's frame is chosen.  And the selected window obviously
> changes because 'read-string' (and in further consequence read_minibuf)
> restores the window selected before the minibuffer window was selected.
>
> Concludingly, the current version works when 'window-in-direction'
> returns a suitable window and may fail miserably otherwise.

I tried to reproduce your recipe, but I couldn't. Likely, I have a too
simple minded use of different Emacs windows. And a not customized mode
line. Note, that I'm not familiar with the windows machinery in Emacs at
all.

> The attached patch fixes the problem here.  But I still think that the
> icon should appear in the prompt of 'read-passwd' itself.

Please install your patch (you might ask Eli whether it is good for the
emacs-30 branch). And as I said in the other message, the mode line is
the place to give information about the active buffer, including the minibuffer.

Thanks a lot for investigating it!

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Michael

 > I tried to reproduce your recipe, but I couldn't. Likely, I have a too
 > simple minded use of different Emacs windows. And a not customized mode
 > line. Note, that I'm not familiar with the windows machinery in Emacs at
 > all.

No customized mode line is needed.  Either with emacs -Q evaluate

(progn
   (set-window-parameter nil 'no-other-window t)
   (with-selected-window (split-window nil -5)
     (set-window-buffer nil (get-buffer-create "*text*"))
     (set-window-parameter nil 'no-other-window t)
     (read-passwd "PWD:")))

or with

emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))

evaluate via M-:

(with-selected-window (get-buffer-window "*scratch*" t)
   (with-selected-window (split-window nil -5)
     (set-window-buffer nil (get-buffer-create "*text*"))
     (read-passwd "PWD:")))

In either case it's important that (1) there is no suitable window
"above" and (2) 'minibuffer-selected-window' doesn't return the largest
window on its frame.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Eli Zaretskii]

> Cc: 73768@debbugs.gnu.org
> Date: Sun, 13 Oct 2024 16:19:13 +0200
> From:  Michael Albinus via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Please install your patch (you might ask Eli whether it is good for the
> emacs-30 branch).

If it solves the problem, it's okay for emacs-30.

Thanks.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

> Hi Michael

Hi Martin,

> No customized mode line is needed.  Either with emacs -Q evaluate
>
> (progn
>   (set-window-parameter nil 'no-other-window t)
>   (with-selected-window (split-window nil -5)
>     (set-window-buffer nil (get-buffer-create "*text*"))
>     (set-window-parameter nil 'no-other-window t)
>     (read-passwd "PWD:")))

Thanks! With this recipe, I can reproduce the problem. And your patch
fixes it :-)

Please install in the emacs-30 branch.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

 > Thanks! With this recipe, I can reproduce the problem. And your patch
 > fixes it :-)
 >
 > Please install in the emacs-30 branch.

Hold your horses.  First try another recipe: With emacs -Q do C-x 5 2
followed by C-x b to show *Messages* in either of the frames, read a
password, do C-x 5 o and type C-g.  One of the two buffers has the eye
icon the other has the botched mode line.  I'm using the more accurate
patch attached now to hopefully fix that too.  Note that whenever one
switches frames, the mode line showing the eye will always stay with the
initially chosen buffer and not the one where the minibuffer window is.

But there are other scenarios that don't seem right.  While reading one
password ask for reading another one.  You now should see two eye icons
which, however, cannot be attributed to any of the two read operations.
Clicking on either of them will toggle visibility of the password.  If
you don't want to show the password for the first operation but you do
want to show it for the second operation, then when the second operation
terminates you will see the password for the first one.  Same for typing
TAB in the minibuffer.  Resetting 'read-passwd--hide-password' when
terminating any read password operation might fix that.

Likely password visibility should be a property of the specific reading
process (aka as buffer-local value of 'read-passwd--hide-password') and
not something global.  But then we would also have to change the first
icon of the mode line too and doing that is more tricky.  Currently, all
eye icons are the same because 'read-passwd--mode-line-icon' has the
same value for them.

Probably it might be better to have only one eye icon on any mode line
informing about the visibility of the innermost minibuffer's password
status.  But this becomes tricky when terminating a password operation
because we would have to internally (on the C-level) scan all
minibuffers to find out whether there's another password operation in
progress and remove the eye icon only if there's none.

Otherwise we could try giving only the first eye icon the dynamic eval
property and have all other icons statically have the hide password
property.  Less tricky - we'd only have to hide a password when asking
for another one.

Obviously, we could also inhibit asking for a second password.  But I
suppose this was always possible and would imply an incompatible change.

If anyone has a good idea what to do here, please speak up.

Thanks, martin

[-- Attachment #2: read-passwd.diff --]
[-- Type: text/x-patch, Size: 1857 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..f71393721cb 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2474,7 +2474,7 @@ read-passwd--mode-line-icon
   "Propertized mode line icon for showing/hiding passwords.")
 
 (defvar read-passwd--hide-password t
-  "Toggle whether password should be hidden in minubuffer.")
+  "Toggle whether password should be hidden in minibuffer.")
 
 (defun read-passwd--hide-password ()
   "Make password in minibuffer hidden or visible."
@@ -2534,13 +2534,14 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"
 
-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  ;; Stolen from `eldoc-minibuffer-message'.
+  (unless (buffer-live-p read-passwd--mode-line-buffer)
+    (setq read-passwd--mode-line-buffer
+          (window-buffer
+           (or (window-in-direction 'above (minibuffer-window))
+	       (minibuffer-selected-window)
+	       (get-largest-window)))))
 
   (if read-passwd-mode
       (with-current-buffer read-passwd--mode-line-buffer
@@ -2551,8 +2552,10 @@ read-passwd-mode
 	              mode-line-format))))
     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
-      (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+      (when (equal (car mode-line-format)
+		   '(:eval read-passwd--mode-line-icon))
+        (setq mode-line-format (cdr mode-line-format))))
+    (setq read-passwd--mode-line-buffer nil))
 
   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

> Hold your horses.  First try another recipe: With emacs -Q do C-x 5 2
> followed by C-x b to show *Messages* in either of the frames, read a
> password, do C-x 5 o and type C-g.  One of the two buffers has the eye
> icon the other has the botched mode line.  I'm using the more accurate
> patch attached now to hopefully fix that too.  Note that whenever one
> switches frames, the mode line showing the eye will always stay with the
> initially chosen buffer and not the one where the minibuffer window is.

Thanks for this. As said I'm a n00b wrt to Emacs window management, so
pls install your patch since it makes the situation better.

> But there are other scenarios that don't seem right.  While reading one
> password ask for reading another one.  You now should see two eye icons
> which, however, cannot be attributed to any of the two read operations.
> Clicking on either of them will toggle visibility of the password.  If
> you don't want to show the password for the first operation but you do
> want to show it for the second operation, then when the second operation
> terminates you will see the password for the first one.  Same for typing
> TAB in the minibuffer.  Resetting 'read-passwd--hide-password' when
> terminating any read password operation might fix that.
>
> Likely password visibility should be a property of the specific reading
> process (aka as buffer-local value of 'read-passwd--hide-password') and
> not something global.  But then we would also have to change the first
> icon of the mode line too and doing that is more tricky.  Currently, all
> eye icons are the same because 'read-passwd--mode-line-icon' has the
> same value for them.
>
> Probably it might be better to have only one eye icon on any mode line
> informing about the visibility of the innermost minibuffer's password
> status.  But this becomes tricky when terminating a password operation
> because we would have to internally (on the C-level) scan all
> minibuffers to find out whether there's another password operation in
> progress and remove the eye icon only if there's none.
>
> Otherwise we could try giving only the first eye icon the dynamic eval
> property and have all other icons statically have the hide password
> property.  Less tricky - we'd only have to hide a password when asking
> for another one.
>
> Obviously, we could also inhibit asking for a second password.  But I
> suppose this was always possible and would imply an incompatible change.

I haven't seen any application in the wild which offers cascaded
password input. What would be the use case for this? My feeling is, that
this is a misuse. We might not forbid it, but we might show a warning.

> If anyone has a good idea what to do here, please speak up.

Sigh.

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Martin,

>> If anyone has a good idea what to do here, please speak up.
>
> Sigh.

I gave it another try. The appended patch dosn't add the password icon
to mode-line-format, but to global-mode-string. By this, we don't bother
to find the corresponding window to a given minibuffer.

This doesn't solve the problem of cascaded password input, but that's
another game.

WDYT?

>> Thanks, martin

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 4318 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..11ddc4e25ca 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2467,14 +2467,11 @@ read-passwd--hide-password-icon
   :version "30.1"
   :help-echo "mouse-1: Toggle password visibility")

-(defvar read-passwd--mode-line-buffer nil
-  "Buffer to modify `mode-line-format' for showing/hiding passwords.")
-
 (defvar read-passwd--mode-line-icon nil
   "Propertized mode line icon for showing/hiding passwords.")

 (defvar read-passwd--hide-password t
-  "Toggle whether password should be hidden in minubuffer.")
+  "Toggle whether password should be hidden in minibuffer.")

 (defun read-passwd--hide-password ()
   "Make password in minibuffer hidden or visible."
@@ -2492,30 +2489,25 @@ read-passwd-toggle-visibility
   "Toggle minibuffer contents visibility.
 Adapt also mode line."
   (interactive)
-  (let ((win (active-minibuffer-window)))
-    (unless win (error "No active minibuffer"))
-    ;; FIXME: In case of a recursive minibuffer, this may select the wrong
-    ;; mini-buffer.
-    (with-current-buffer (window-buffer win)
-      (setq read-passwd--hide-password (not read-passwd--hide-password))
-      (with-current-buffer read-passwd--mode-line-buffer
-        (setq read-passwd--mode-line-icon
-              `(:propertize
-                ,(if icon-preference
-                     (icon-string
-                      (if read-passwd--hide-password
-                          'read-passwd--show-password-icon
-                        'read-passwd--hide-password-icon))
-                   "")
-                mouse-face mode-line-highlight
-                keymap
-                ,(eval-when-compile
-                   (let ((map (make-sparse-keymap)))
-                     (define-key map [mode-line mouse-1]
-                                 #'read-passwd-toggle-visibility)
-                     map))))
-        (force-mode-line-update))
-      (read-passwd--hide-password))))
+  (when (minibufferp)
+    (setq read-passwd--hide-password (not read-passwd--hide-password))
+    (setq read-passwd--mode-line-icon
+          `(:propertize
+            ,(if icon-preference
+                 (icon-string
+                  (if read-passwd--hide-password
+                      'read-passwd--show-password-icon
+                    'read-passwd--hide-password-icon))
+               "")
+            mouse-face mode-line-highlight
+            keymap
+            ,(eval-when-compile
+               (let ((map (make-sparse-keymap)))
+                 (define-key map [mode-line mouse-1]
+                             #'read-passwd-toggle-visibility)
+                 map))))
+    (force-mode-line-update 'all)
+    (read-passwd--hide-password)))

 (defvar read-passwd-map
   ;; BEWARE: `defconst' would purecopy it, breaking the sharing with
@@ -2534,25 +2526,18 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"

-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  (or global-mode-string (setq global-mode-string '("")))

-  (if read-passwd-mode
-      (with-current-buffer read-passwd--mode-line-buffer
+  (let ((mode-string '(:eval read-passwd--mode-line-icon)))
+    (if read-passwd-mode
         ;; Add `read-passwd--mode-line-icon'.
-        (when (listp mode-line-format)
-          (setq mode-line-format
-                (cons '(:eval read-passwd--mode-line-icon)
-	              mode-line-format))))
-    (with-current-buffer read-passwd--mode-line-buffer
+        (or (member mode-string global-mode-string)
+            (setq global-mode-string
+	          (append global-mode-string (list mode-string))))
       ;; Remove `read-passwd--mode-line-icon'.
-      (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+      (setq global-mode-string
+	    (delete mode-string global-mode-string))))

   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > I haven't seen any application in the wild which offers cascaded
 > password input. What would be the use case for this? My feeling is, that
 > this is a misuse. We might not forbid it, but we might show a warning.

Then why does 'read-passwd' deliberately offer it via

             (let ((enable-recursive-minibuffers t)

If it's a misuse, it should rather bind it to nil.

Another mystery: After

commit de88ed8646383301f20f8340e13d43e5918310fa
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Sun Oct 13 05:15:18 2019 +0200

     Hide passwords more in `read-password'

     * lisp/subr.el (read-password--hide-password): Factor out (bug#17127).
     (read-passwd): Use it in post-command-hook instead of
     after-change-functions to ensure that we hide the password no
     matter what inserts text.

entering 'read-passwd' does

             (add-hook 'post-command-hook #'read-passwd--hide-password nil t))

while exiting still does

               ;; Not sure why but it seems that there might be cases where the
               ;; minibuffer is not always properly reset later on, so undo
               ;; whatever we've done here (bug#11392).
               (remove-hook 'after-change-functions
                            #'read-passwd--hide-password 'local)

If that fix of Bug#11392 (or Bug#11397) were really needed, then we
should remove read-password--hide-password from 'post-command-hook' now.
But there were no bug reports in the last four years so probably that
bug was fixed by other means.  In either case the current 'remove-hook'
is a noop now.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > I gave it another try. The appended patch dosn't add the password icon
 > to mode-line-format, but to global-mode-string. By this, we don't bother
 > to find the corresponding window to a given minibuffer.

I think this is the best idea I've seen so far here.  Users can switch
frames and windows in some arbitrary manner and the eye is always there.
As long as you are satisfied with seeing the eye at the end of the mode
line which for narrow windows can be hidden.  Have you considered using
'mode-line-front-space' instead?

 > This doesn't solve the problem of cascaded password input, but that's
 > another game.

I would append another eye just as the current code does and remove one
when the password has been read.  And whenever a password has been read
make sure to hide passwords.  Ideally, we could also give at most one of
the eyes (the left- or rightmost one) the visible status.  And to make
everything foolproof we could make sure all eyes are removed as soon as
'minibuffer-depth' drops to zero.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

 > As long as you are satisfied with seeing the eye at the end of the mode
 > line which for narrow windows can be hidden.  Have you considered using
 > 'mode-line-front-space' instead?

I tried that now and it seems to work well here.  Have a look.

martin

[-- Attachment #2: read-passwd.diff --]
[-- Type: text/x-patch, Size: 4473 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..274578185d2 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2467,14 +2467,11 @@ read-passwd--hide-password-icon
   :version "30.1"
   :help-echo "mouse-1: Toggle password visibility")
 
-(defvar read-passwd--mode-line-buffer nil
-  "Buffer to modify `mode-line-format' for showing/hiding passwords.")
-
 (defvar read-passwd--mode-line-icon nil
   "Propertized mode line icon for showing/hiding passwords.")
 
 (defvar read-passwd--hide-password t
-  "Toggle whether password should be hidden in minubuffer.")
+  "Toggle whether password should be hidden in minibuffer.")
 
 (defun read-passwd--hide-password ()
   "Make password in minibuffer hidden or visible."
@@ -2492,30 +2489,25 @@ read-passwd-toggle-visibility
   "Toggle minibuffer contents visibility.
 Adapt also mode line."
   (interactive)
-  (let ((win (active-minibuffer-window)))
-    (unless win (error "No active minibuffer"))
-    ;; FIXME: In case of a recursive minibuffer, this may select the wrong
-    ;; mini-buffer.
-    (with-current-buffer (window-buffer win)
-      (setq read-passwd--hide-password (not read-passwd--hide-password))
-      (with-current-buffer read-passwd--mode-line-buffer
-        (setq read-passwd--mode-line-icon
-              `(:propertize
-                ,(if icon-preference
-                     (icon-string
-                      (if read-passwd--hide-password
-                          'read-passwd--show-password-icon
-                        'read-passwd--hide-password-icon))
-                   "")
-                mouse-face mode-line-highlight
-                keymap
-                ,(eval-when-compile
-                   (let ((map (make-sparse-keymap)))
-                     (define-key map [mode-line mouse-1]
-                                 #'read-passwd-toggle-visibility)
-                     map))))
-        (force-mode-line-update))
-      (read-passwd--hide-password))))
+  (when (minibufferp)
+    (setq read-passwd--hide-password (not read-passwd--hide-password))
+    (setq read-passwd--mode-line-icon
+          `(:propertize
+            ,(if icon-preference
+                 (icon-string
+                  (if read-passwd--hide-password
+                      'read-passwd--show-password-icon
+                    'read-passwd--hide-password-icon))
+               "")
+            mouse-face mode-line-highlight
+            keymap
+            ,(eval-when-compile
+               (let ((map (make-sparse-keymap)))
+                 (define-key map [mode-line mouse-1]
+                             #'read-passwd-toggle-visibility)
+                 map))))
+    (force-mode-line-update 'all)
+    (read-passwd--hide-password)))
 
 (defvar read-passwd-map
   ;; BEWARE: `defconst' would purecopy it, breaking the sharing with
@@ -2534,26 +2526,25 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"
 
-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
-
-  (if read-passwd-mode
-      (with-current-buffer read-passwd--mode-line-buffer
-        ;; Add `read-passwd--mode-line-icon'.
-        (when (listp mode-line-format)
-          (setq mode-line-format
-                (cons '(:eval read-passwd--mode-line-icon)
-	              mode-line-format))))
-    (with-current-buffer read-passwd--mode-line-buffer
-      ;; Remove `read-passwd--mode-line-icon'.
-      (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+  (setq read-passwd--hide-password nil)
+  (or mode-line-front-space
+      (setq mode-line-front-space
+	    '(:eval (if (display-graphic-p) " " "-"))))
 
+  (let ((mode-line-string '(:eval read-passwd--mode-line-icon)))
+    (cond
+     (read-passwd-mode
+      ;; Add `read-passwd--mode-line-icon'.
+      (setq mode-line-front-space
+	    (cons mode-line-string mode-line-front-space)))
+     ;; Remove `read-passwd--mode-line-icon'.
+     ((equal (car mode-line-front-space)
+	     mode-line-string)
+      (setq mode-line-front-space (cdr mode-line-front-space)))
+     (t
+      (delete mode-line-string mode-line-front-space))))
+
+  (force-mode-line-update 'all)
   (when read-passwd-mode
     (read-passwd-toggle-visibility)))
 

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

>> As long as you are satisfied with seeing the eye at the end of the mode
>> line which for narrow windows can be hidden.  Have you considered using
>> 'mode-line-front-space' instead?
>
> I tried that now and it seems to work well here.  Have a look.

Unfortunately, it doesn't seem to work with your recipe

--8<---------------cut here---------------start------------->8---
emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))

evaluate via M-:

(with-selected-window (get-buffer-window "*scratch*" t)
  (with-selected-window (split-window nil -5)
    (set-window-buffer nil (get-buffer-create "*text*"))
    (read-passwd "PWD:")))
--8<---------------cut here---------------end--------------->8---

I also believe that modifying mode-line-front-space is rather a
hack. Nobody seems to do this in Emacs' lisp/ directory. And the
documentation of this variable doesn't encourage its random
modification.

I propose to apply the patch with modifying global-mode-string to
emacs-30. A more sophisticated solution could be in the master branch
then. Yes, the password icon could be invisible due to short window
width, but this might be acceptable.

WDYT?

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

>> This doesn't solve the problem of cascaded password input, but that's
>> another game.
>
> I would append another eye just as the current code does and remove one
> when the password has been read.  And whenever a password has been read
> make sure to hide passwords.  Ideally, we could also give at most one of
> the eyes (the left- or rightmost one) the visible status.  And to make
> everything foolproof we could make sure all eyes are removed as soon as
> 'minibuffer-depth' drops to zero.

I believe using cascade password input is a misuse. I cannot think of
any use-case.

And the situation isn't that bad. After finishing the inner passwd-read,
the password icon disappears. So what, everything else still works. We
don't need to support this edge case.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

>> I haven't seen any application in the wild which offers cascaded
>> password input. What would be the use case for this? My feeling is, that
>> this is a misuse. We might not forbid it, but we might show a warning.
>
> Then why does 'read-passwd' deliberately offer it via
>
>             (let ((enable-recursive-minibuffers t)
>
> If it's a misuse, it should rather bind it to nil.

This is not offered for cascaded password input. It is enabled for
recursive use of the minibuffer for any purpose. For example, while
typing the password, you need to check somehing else via 'M-:'. This is
possible now.

> Another mystery: After
>
> commit de88ed8646383301f20f8340e13d43e5918310fa
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Date:   Sun Oct 13 05:15:18 2019 +0200
>
>     Hide passwords more in `read-password'
>
>     * lisp/subr.el (read-password--hide-password): Factor out (bug#17127).
>     (read-passwd): Use it in post-command-hook instead of
>     after-change-functions to ensure that we hide the password no
>     matter what inserts text.
>
> entering 'read-passwd' does
>
>             (add-hook 'post-command-hook #'read-passwd--hide-password nil t))
>
> while exiting still does
>
>               ;; Not sure why but it seems that there might be cases where the
>               ;; minibuffer is not always properly reset later on, so undo
>               ;; whatever we've done here (bug#11392).
>               (remove-hook 'after-change-functions
>                            #'read-passwd--hide-password 'local)
>
> If that fix of Bug#11392 (or Bug#11397) were really needed, then we
> should remove read-password--hide-password from 'post-command-hook' now.
> But there were no bug reports in the last four years so probably that
> bug was fixed by other means.  In either case the current 'remove-hook'
> is a noop now.

Hmm, yes. While adding read-passwd-mode, I didn't check all settings in
read-passwd. Likely these settings are superfluous now. I will check in
master once the other patch has arrived it.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 >> I tried that now and it seems to work well here.  Have a look.
 >
 > Unfortunately, it doesn't seem to work with your recipe

I tried only a few standard scenarios.

 > I also believe that modifying mode-line-front-space is rather a
 > hack. Nobody seems to do this in Emacs' lisp/ directory. And the
 > documentation of this variable doesn't encourage its random
 > modification.

Agreed.

 > I propose to apply the patch with modifying global-mode-string to
 > emacs-30. A more sophisticated solution could be in the master branch
 > then. Yes, the password icon could be invisible due to short window
 > width, but this might be acceptable.

Users can always compact mode lines if needed.

 > WDYT?

I think you should check it in.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

before.> I gave it another try. The appended patch dosn't add the password icon
 > to mode-line-format, but to global-mode-string. By this, we don't bother
 > to find the corresponding window to a given minibuffer.

One minor problem exists when using a stand-alone minibuffer frame.
Clicking the icons never works because they appear on another frame and
when you move the mouse there

   (when (minibufferp)

fails.  Basically, clicks work now iff the minibuffer window is selected
and not if the minibuffer window is active as previously.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

> One minor problem exists when using a stand-alone minibuffer frame.
> Clicking the icons never works because they appear on another frame and
> when you move the mouse there
>
>   (when (minibufferp)
>
> fails.  Basically, clicks work now iff the minibuffer window is selected
> and not if the minibuffer window is active as previously.

I've seen, yes. I have no solution now.

There are further problems like

--8<---------------cut here---------------start------------->8---
emacs -Q --eval '(read-passwd "PWD: ")'
i n p u t TAB
M-: x x x
--8<---------------cut here---------------end--------------->8---

The inner minibuffer, showing "xxx", toggles its visibility when
clicking the password icon. I've extended the patch to check for
read-passwd-mode in local-minor-modes.

There will be other issues. But I still believe we shall push the patch
using global-mode-string to emacs-30, because it fixes some of the
problems. The rest must wait for the master branch.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Martin,

>> One minor problem exists when using a stand-alone minibuffer frame.
>> Clicking the icons never works because they appear on another frame and
>> when you move the mouse there
>>
>>   (when (minibufferp)
>>
>> fails.  Basically, clicks work now iff the minibuffer window is selected
>> and not if the minibuffer window is active as previously.
>
> I've seen, yes. I have no solution now.

I've modified the patch, this case seems to be covered now.

> There are further problems like
>
> emacs -Q --eval '(read-passwd "PWD: ")'
> i n p u t TAB
> M-: x x x
>
> The inner minibuffer, showing "xxx", toggles its visibility when
> clicking the password icon. I've extended the patch to check for
> read-passwd-mode in local-minor-modes.

This fix is also included in the patch.

Could you pls check whether you see further annoyances? Otherwise, if
you give me a go, I'll push to emacs-30.

>> martin

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3214 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..557d360bc6a 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2467,14 +2467,11 @@ read-passwd--hide-password-icon
   :version "30.1"
   :help-echo "mouse-1: Toggle password visibility")

-(defvar read-passwd--mode-line-buffer nil
-  "Buffer to modify `mode-line-format' for showing/hiding passwords.")
-
 (defvar read-passwd--mode-line-icon nil
   "Propertized mode line icon for showing/hiding passwords.")

 (defvar read-passwd--hide-password t
-  "Toggle whether password should be hidden in minubuffer.")
+  "Toggle whether password should be hidden in minibuffer.")

 (defun read-passwd--hide-password ()
   "Make password in minibuffer hidden or visible."
@@ -2497,8 +2494,8 @@ read-passwd-toggle-visibility
     ;; FIXME: In case of a recursive minibuffer, this may select the wrong
     ;; mini-buffer.
     (with-current-buffer (window-buffer win)
-      (setq read-passwd--hide-password (not read-passwd--hide-password))
-      (with-current-buffer read-passwd--mode-line-buffer
+      (when (memq 'read-passwd-mode local-minor-modes)
+        (setq read-passwd--hide-password (not read-passwd--hide-password))
         (setq read-passwd--mode-line-icon
               `(:propertize
                 ,(if icon-preference
@@ -2514,8 +2511,8 @@ read-passwd-toggle-visibility
                      (define-key map [mode-line mouse-1]
                                  #'read-passwd-toggle-visibility)
                      map))))
-        (force-mode-line-update))
-      (read-passwd--hide-password))))
+        (force-mode-line-update 'all)
+        (read-passwd--hide-password)))))

 (defvar read-passwd-map
   ;; BEWARE: `defconst' would purecopy it, breaking the sharing with
@@ -2534,25 +2531,18 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"

-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  (or global-mode-string (setq global-mode-string '("")))

-  (if read-passwd-mode
-      (with-current-buffer read-passwd--mode-line-buffer
+  (let ((mode-string '(:eval read-passwd--mode-line-icon)))
+    (if read-passwd-mode
         ;; Add `read-passwd--mode-line-icon'.
-        (when (listp mode-line-format)
-          (setq mode-line-format
-                (cons '(:eval read-passwd--mode-line-icon)
-	              mode-line-format))))
-    (with-current-buffer read-passwd--mode-line-buffer
+        (or (member mode-string global-mode-string)
+            (setq global-mode-string
+	          (append global-mode-string (list mode-string))))
       ;; Remove `read-passwd--mode-line-icon'.
-      (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+      (setq global-mode-string
+	    (delete mode-string global-mode-string))))

   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > There will be other issues. But I still believe we shall push the patch
 > using global-mode-string to emacs-30, because it fixes some of the
 > problems. The rest must wait for the master branch.

I see no more problems here so please push it (IIRC you intended to add
some explanations to the Emacs manual too).

Thanks, martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

>> There will be other issues. But I still believe we shall push the patch
>> using global-mode-string to emacs-30, because it fixes some of the
>> problems. The rest must wait for the master branch.
>
> I see no more problems here so please push it (IIRC you intended to add
> some explanations to the Emacs manual too).

Done. Pushed to emacs-30, and merged with master. Now we can continue
with the other problems you've mentioned.

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

>> I propose to apply the patch with modifying global-mode-string to
>> emacs-30. A more sophisticated solution could be in the master branch
>> then. Yes, the password icon could be invisible due to short window
>> width, but this might be acceptable.
>
> Users can always compact mode lines if needed.

Or move 'global-mode-string' (‘%M’) somewhere else to the front of 'mode-line-format'.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Done. Pushed to emacs-30, and merged with master.

Great.

 > Now we can continue
 > with the other problems you've mentioned.

The only difficult one I see is that of whether and how to display an
icon when returning to a previous invocation.  But I certainly won't
insist on doing something in this regard.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

> The only difficult one I see is that of whether and how to display an
> icon when returning to a previous invocation.  But I certainly won't
> insist on doing something in this regard.

As said before, I don't regard it as our responsibility to handle this
case. So I'd rather do nothing here.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > As said before, I don't regard it as our responsibility to handle this
 > case. So I'd rather do nothing here.

So I think you should just remove the entire

               ;; Not sure why but it seems that there might be cases where the
               ;; minibuffer is not always properly reset later on, so undo
               ;; whatever we've done here (bug#11392).
               (remove-hook 'after-change-functions
                            #'read-passwd--hide-password 'local)

on master and mark the bug as done.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

>> As said before, I don't regard it as our responsibility to handle this
>> case. So I'd rather do nothing here.
>
> So I think you should just remove the entire
>
>               ;; Not sure why but it seems that there might be cases where the
>               ;; minibuffer is not always properly reset later on, so undo
>               ;; whatever we've done here (bug#11392).
>               (remove-hook 'after-change-functions
>                            #'read-passwd--hide-password 'local)
>
> on master and mark the bug as done.

Yes, I'm just reviewing the settings in read-passwd which I have ignored
so far. Since we're using the minor mode read-passwd-mode in the
minibuffer now, I expect that all these settings go away when the
function returns.

To be tested.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Version: 30.1

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Martin,

>> entering 'read-passwd' does
>>
>>             (add-hook 'post-command-hook #'read-passwd--hide-password nil t))
>>
>> while exiting still does
>>
>>               ;; Not sure why but it seems that there might be cases where the
>>               ;; minibuffer is not always properly reset later on, so undo
>>               ;; whatever we've done here (bug#11392).
>>               (remove-hook 'after-change-functions
>>                            #'read-passwd--hide-password 'local)
>>
>> If that fix of Bug#11392 (or Bug#11397) were really needed, then we
>> should remove read-password--hide-password from 'post-command-hook' now.
>> But there were no bug reports in the last four years so probably that
>> bug was fixed by other means.  In either case the current 'remove-hook'
>> is a noop now.
>
> Hmm, yes. While adding read-passwd-mode, I didn't check all settings in
> read-passwd. Likely these settings are superfluous now. I will check in
> master once the other patch has arrived it.

Finally, I've done as you proposed: Remove `read-password--hide-password'
from `post-command-hook'. While playing with the recipe of bug#11397,
sometimes (not always) the minibuffer was corrupted as hiding the
text. So I believe this is the safest change.

Pushed to emacs-30. And closing this bug, now. Thanks again for your
patient discussion the issue!

>> martin

Best regards, Michael.