file-name-shadow-mode

file-name-shadow-mode

[David Kastrup]

Hi, if I remember correctly, we had agreed a long time ago to make
file-name-shadow-mode active by default for the release.

Seemingly, this has not happened yet.  I think the discussion petered
off after somebody asked how one could actually auto-enable some minor
mode.

I am using this all of the time.  Any comment?  I find this _very_
useful not only for beginners.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Matt Hodges]

>>>>> David Kastrup writes:

 > Hi, if I remember correctly, we had agreed a long time ago to make
 > file-name-shadow-mode active by default for the release.

 > Seemingly, this has not happened yet.  I think the discussion
 > petered off after somebody asked how one could actually auto-enable
 > some minor mode.

 > I am using this all of the time.  Any comment?  I find this _very_
 > useful not only for beginners.

I agree that it's very useful.  Furthermore, in a 256-colour xterm

    (setq file-name-shadow-tty-properties file-name-shadow-properties)

works perfectly well.  I checked a 16-colour xterm, and needed to make
the foreground colour of file-name-shadow face darker (gray30) for it
to be easily-distinguishable from a white background.

Matt

Re: file-name-shadow-mode

[Miles Bader]

On Thu, 17 Mar 2005 12:16:23 +0000, Matt Hodges <MPHodges@member.fsf.org> wrote:
> I agree that it's very useful.  Furthermore, in a 256-colour xterm
> 
>     (setq file-name-shadow-tty-properties file-name-shadow-properties)
> 
> works perfectly well.  I checked a 16-colour xterm, and needed to make
> the foreground colour of file-name-shadow face darker (gray30) for it
> to be easily-distinguishable from a white background.

It might be more accurate to call it `file-name-shadow-low-color-properties'.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: file-name-shadow-mode

[Miles Bader]

On Thu, 17 Mar 2005 10:16:37 +0100, David Kastrup <dak@gnu.org> wrote:
> Hi, if I remember correctly, we had agreed a long time ago to make
> file-name-shadow-mode active by default for the release.

Yes

> Seemingly, this has not happened yet.  I think the discussion petered
> off after somebody asked how one could actually auto-enable some minor
> mode.

That was me...

Does anyone have a quick answer to that question?  Otherwise I suppose
I'll try to find out (hmmm; what's another enabled-by-default global
minor mode?).

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: file-name-shadow-mode

[Nick Roberts]

 > > Seemingly, this has not happened yet.  I think the discussion petered
 > > off after somebody asked how one could actually auto-enable some minor
 > > mode.
 > 
 > That was me...
 > 
 > Does anyone have a quick answer to that question?  Otherwise I suppose
 > I'll try to find out (hmmm; what's another enabled-by-default global
 > minor mode?).

Put it in startup.el? Thats where tooltip-mode is.

Nick

Re: file-name-shadow-mode

[Luc Teirlinck]

Miles Bader wrote:

   > Seemingly, this has not happened yet.  I think the discussion petered
   > off after somebody asked how one could actually auto-enable some minor
   > mode.

   That was me...

   Does anyone have a quick answer to that question?

I believe that the reason you have problems is that define-minor-mode
uses `custom-initialize-default' as the Custom :initialize function.
That is correct if you want the minor mode disabled by default, but
not if you want it enabled by default.  Then you need to use
`custom-initialize-set'.  Passing an explicit :initialize keyword to
`define-minor-mode' does not seem to help: it gets ignored.  I believe
that `define-minor-mode' should use `custom-initialize-set' as the
:initialize function if :init-value is non-nil.  Then the patches
below would work.  _Currently they do not._  An alternate patch that
does work right now is to call `custom-reevaluate-setting' in
startup.el, but to me, that seems ugly.

===File ~/src-Makefile.in-diff==============================
*** Makefile.in	30 Nov 2004 16:57:14 -0600	1.307
--- Makefile.in	17 Mar 2005 20:58:22 -0600	
***************
*** 779,784 ****
--- 779,785 ----
  	${lispsource}emacs-lisp/timer.elc \
  	${lispsource}vc-hooks.elc \
  	${lispsource}ediff-hook.elc \
+ 	${lispsource}rfn-eshadow.elc \
  	VMS_SUPPORT \
  	MSDOS_SUPPORT \
  	WINNT_SUPPORT \
***************
*** 870,875 ****
--- 871,877 ----
  	../lisp/emacs-lisp/timer.elc \
  	../lisp/vc-hooks.elc \
  	../lisp/ediff-hook.elc \
+ 	../lisp/rfn-eshadow.elc \
  	../lisp/widget.elc \
  	../lisp/window.elc \
  	../lisp/version.el
============================================================

===File ~/loadup-diff=======================================
*** loadup.el	27 Dec 2004 15:02:07 -0600	1.135
--- loadup.el	17 Mar 2005 21:50:40 -0600	
***************
*** 192,197 ****
--- 192,198 ----
  
  (load "vc-hooks")
  (load "ediff-hook")
+ (load "rfn-eshadow")
  (message "%s" (garbage-collect))
  
  ;If you want additional libraries to be preloaded and their
============================================================

===File ~/rfn-eshadow-diff==================================
*** rfn-eshadow.el	02 Sep 2003 07:36:02 -0500	1.12
--- rfn-eshadow.el	17 Mar 2005 21:45:48 -0600	
***************
*** 207,212 ****
--- 207,214 ----
  With prefix argument ARG, turn on if positive, otherwise off.
  Returns non-nil if the new state is enabled."
    :global t
+   :init-value t
    :group 'minibuffer
    (if file-name-shadow-mode
        ;; Enable the mode
============================================================

Re: file-name-shadow-mode

[Stefan Monnier]

> I believe that the reason you have problems is that define-minor-mode
> uses `custom-initialize-default' as the Custom :initialize function.
> That is correct if you want the minor mode disabled by default, but
> not if you want it enabled by default.  Then you need to use
> `custom-initialize-set'.  Passing an explicit :initialize keyword to
> `define-minor-mode' does not seem to help: it gets ignored.  I believe
> that `define-minor-mode' should use `custom-initialize-set' as the
> :initialize function if :init-value is non-nil.  Then the patches
> below would work.  _Currently they do not._  An alternate patch that
> does work right now is to call `custom-reevaluate-setting' in
> startup.el, but to me, that seems ugly.

Assuming it's still considered a good idea to make sure that loading a file
does not have any other side effect, I think it's better to keep
the :init-value at nil, and simply add a call to file-name-shadow-mode
somewhere (either in startup, or loadup).


        Stefan

Re: file-name-shadow-mode

[Luc Teirlinck]

Stefan Monnier wrote:

   Assuming it's still considered a good idea to make sure that loading a file
   does not have any other side effect,

I personally very much believe it is a bad idea.  _However_ it is the
_general_ default :initialize function (`custom-initialize-reset')
that has this problem and _not_ `custom-initialize-set' (note the
missing `re').  It is `custom-initialize-set' that I propose to use if
:init-value is non-nil.

By the way, quoting from my original message:

   Passing an explicit :initialize keyword to
   `define-minor-mode' does not seem to help: it gets ignored.

Actually, no.  After macroexpansion, :initialize occurs _twice_ in the
defcustom, once as `custom-initialize-default' and once as
`custom-initialize-set', but then Custom apparently ignores the latter.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Luc Teirlinck]

>From my previous message:

    I personally very much believe it is a bad idea.  _However_ it is the
    _general_ default :initialize function (`custom-initialize-reset')
    that has this problem and _not_ `custom-initialize-set' (note the
    missing `re').

Actually, I was confused here: `custom-initialize-set' does have a
side effect when loading the file, but, unlike the very problematic
default `custom-initialize-reset' it will not override user
customizations.  (These are the two problems I confused.)

Setting :init-value non-nil for a minor-mode already has a side effect
and a pretty confusing one, since the variable and the function are
now out of sync.  Unless the minor mode requires no :set function, in
which case the minor mode is enabled and thus there is a side effect.
It seems only appropriate for pre-loaded files and I propose to make
the file in question pre-loaded.

   I think it's better to keep the :init-value at nil,

If the real default value is t, then the :init-value _needs_ to be t,
for reasons discussed earlier.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Luc Teirlinck]

An alternative to setting :initialize to `custom-initialize-set'
whenever :init-value is non-nil would be to fix `define-minor-mode' to
handle an explicitly passed :initialize keyword correctly.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Richard Stallman]

    Assuming it's still considered a good idea to make sure that loading a file
    does not have any other side effect,

This doesn't apply to preloaded files.  If you reload this file,
it won't alter the option's value.

Re: file-name-shadow-mode

[Luc Teirlinck]

The following four patches enable `file-name-shadow-mode' by default,
preload rfn-eshadow.el and make define-minor-mode accept an
:initialize argument.  (It currently kind of accepts such an argument,
but the result is that the macroexpanded defcustom contains two
:initialize keywords, a bug.)  To make things work correctly, I had to
rearrange define-minor-mode to define the mode function before the
defcustom.  Apparently, this gives no compiler warnings (I checked),
even though the function uses the mode variable, which is, in the new
version, not yet defcustomed.

The patches to loadup.el and src/Makefile.in are identical to those I
sent earlier.

I can install if desired.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	02 Nov 2004 08:26:43 -0600	1.55
--- easy-mmode.el	18 Mar 2005 18:25:25 -0600	
***************
*** 131,150 ****
  	 (hook (intern (concat mode-name "-hook")))
  	 (hook-on (intern (concat mode-name "-on-hook")))
  	 (hook-off (intern (concat mode-name "-off-hook")))
! 	 keyw keymap-sym)
  
      ;; Check keys.
      (while (keywordp (setq keyw (car body)))
        (setq body (cdr body))
        (case keyw
! 	(:init-value (setq init-value (pop body)))
! 	(:lighter (setq lighter (pop body)))
! 	(:global (setq globalp (pop body)))
! 	(:extra-args (setq extra-args (pop body)))
! 	(:group (setq group (nconc group (list :group (pop body)))))
! 	(:require (setq require (pop body)))
! 	(:keymap (setq keymap (pop body)))
! 	(t (push keyw extra-keywords) (push (pop body) extra-keywords))))
  
      (setq keymap-sym (if (and keymap (symbolp keymap)) keymap
  		       (intern (concat mode-name "-map"))))
--- 131,151 ----
  	 (hook (intern (concat mode-name "-hook")))
  	 (hook-on (intern (concat mode-name "-on-hook")))
  	 (hook-off (intern (concat mode-name "-off-hook")))
! 	 keyw keymap-sym initialize)
  
      ;; Check keys.
      (while (keywordp (setq keyw (car body)))
        (setq body (cdr body))
        (case keyw
! 	    (:init-value (setq init-value (pop body)))
! 	    (:lighter (setq lighter (pop body)))
! 	    (:global (setq globalp (pop body)))
! 	    (:extra-args (setq extra-args (pop body)))
! 	    (:group (setq group (nconc group (list :group (pop body)))))
! 	    (:initialize (setq initialize (list :initialize (pop body))))
! 	    (:require (setq require (pop body)))
! 	    (:keymap (setq keymap (pop body)))
! 	    (t (push keyw extra-keywords) (push (pop body) extra-keywords))))
  
      (setq keymap-sym (if (and keymap (symbolp keymap)) keymap
  		       (intern (concat mode-name "-map"))))
***************
*** 155,190 ****
  	    `(:group ',(or (custom-current-group)
  			   (intern (replace-regexp-in-string
  				    "-mode\\'" "" mode-name))))))
  
      `(progn
-        ;; Define the variable to enable or disable the mode.
-        ,(if (not globalp)
- 	    `(progn
- 	       (defvar ,mode ,init-value ,(format "Non-nil if %s is enabled.
- Use the command `%s' to change this variable." pretty-name mode))
- 	       (make-variable-buffer-local ',mode))
- 
- 	  (let ((curfile (or (and (boundp 'byte-compile-current-file)
- 				  byte-compile-current-file)
- 			     load-file-name)))
- 	    `(defcustom ,mode ,init-value
- 	       ,(format "Non-nil if %s is enabled.
- See the command `%s' for a description of this minor-mode.
- Setting this variable directly does not take effect;
- use either \\[customize] or the function `%s'."
- 			pretty-name mode mode)
- 	       :set 'custom-set-minor-mode
- 	       :initialize 'custom-initialize-default
- 	       ,@group
- 	       :type 'boolean
- 	       ,@(cond
- 		  ((not (and curfile require)) nil)
- 		  ((not (eq require t)) `(:require ,require))
- 		  (t `(:require
- 		       ',(intern (file-name-nondirectory
- 				  (file-name-sans-extension curfile))))))
- 	       ,@(nreverse extra-keywords))))
- 
         ;; The actual function.
         (defun ,mode (&optional arg ,@extra-args)
  	 ,(or doc
--- 156,165 ----
  	    `(:group ',(or (custom-current-group)
  			   (intern (replace-regexp-in-string
  				    "-mode\\'" "" mode-name))))))
+     (unless initialize
+       (setq initialize '(:initialize 'custom-initialize-default)))
  
      `(progn
         ;; The actual function.
         (defun ,mode (&optional arg ,@extra-args)
  	 ,(or doc
***************
*** 219,225 ****
  	 ;; Return the new setting.
  	 ,mode)
  
!        ;; Autoloading an easy-mmode-define-minor-mode autoloads
         ;; everything up-to-here.
         :autoload-end
  
--- 194,228 ----
  	 ;; Return the new setting.
  	 ,mode)
  
!        ;; Define the variable to enable or disable the mode.
!        ,(if (not globalp)
! 	    `(progn
! 	       (defvar ,mode ,init-value ,(format "Non-nil if %s is enabled.
! Use the command `%s' to change this variable." pretty-name mode))
! 	       (make-variable-buffer-local ',mode))
! 
! 	  (let ((curfile (or (and (boundp 'byte-compile-current-file)
! 				  byte-compile-current-file)
! 			     load-file-name)))
! 	    `(defcustom ,mode ,init-value
! 	       ,(format "Non-nil if %s is enabled.
! See the command `%s' for a description of this minor-mode.
! Setting this variable directly does not take effect;
! use either \\[customize] or the function `%s'."
! 			pretty-name mode mode)
! 	       :set 'custom-set-minor-mode
! 	       ,@initialize
! 	       ,@group
! 	       :type 'boolean
! 	       ,@(cond
! 		  ((not (and curfile require)) nil)
! 		  ((not (eq require t)) `(:require ,require))
! 		  (t `(:require
! 		       ',(intern (file-name-nondirectory
! 				  (file-name-sans-extension curfile))))))
! 	       ,@(nreverse extra-keywords))))
! 
!       ;; Autoloading an easy-mmode-define-minor-mode autoloads
         ;; everything up-to-here.
         :autoload-end
  
============================================================

===File ~/rfn-eshadow-diff==================================
*** rfn-eshadow.el	02 Sep 2003 07:36:02 -0500	1.12
--- rfn-eshadow.el	18 Mar 2005 17:49:19 -0600	
***************
*** 92,98 ****
  		  (symbol :tag "Property")
  		  (sexp :tag "Value")))))
  
- ;;;###autoload
  (defcustom file-name-shadow-properties
    '(face file-name-shadow field shadow)
    "Properties given to the `shadowed' part of a filename in the minibuffer.
--- 92,97 ----
***************
*** 102,108 ****
    :type file-name-shadow-properties-custom-type
    :group 'minibuffer)
  
- ;;;###autoload
  (defcustom file-name-shadow-tty-properties
    '(before-string "{" after-string "} " field shadow)
    "Properties given to the `shadowed' part of a filename in the minibuffer.
--- 101,106 ----
***************
*** 195,201 ****
  ;;; `define-minor-mode' actually calls the mode-function if the
  ;;; associated variable is non-nil, which requires that all needed
  ;;; functions be already defined.  [This is arguably a bug in d-m-m]
- ;;;###autoload
  (define-minor-mode file-name-shadow-mode
    "Toggle File-Name Shadow mode.
  When active, any part of a filename being read in the minibuffer
--- 193,198 ----
***************
*** 207,212 ****
--- 204,211 ----
  With prefix argument ARG, turn on if positive, otherwise off.
  Returns non-nil if the new state is enabled."
    :global t
+   :init-value t
+   :initialize 'custom-initialize-set
    :group 'minibuffer
    (if file-name-shadow-mode
        ;; Enable the mode
============================================================

===File ~/loadup-diff=======================================
*** loadup.el	27 Dec 2004 15:02:07 -0600	1.135
--- loadup.el	18 Mar 2005 15:15:11 -0600	
***************
*** 192,197 ****
--- 192,198 ----
  
  (load "vc-hooks")
  (load "ediff-hook")
+ (load "rfn-eshadow")
  (message "%s" (garbage-collect))
  
  ;If you want additional libraries to be preloaded and their
============================================================
	
===File ~/src-Makefile.in-diff==============================
*** Makefile.in	30 Nov 2004 16:57:14 -0600	1.307
--- Makefile.in	17 Mar 2005 20:58:22 -0600	
***************
*** 779,784 ****
--- 779,785 ----
  	${lispsource}emacs-lisp/timer.elc \
  	${lispsource}vc-hooks.elc \
  	${lispsource}ediff-hook.elc \
+ 	${lispsource}rfn-eshadow.elc \
  	VMS_SUPPORT \
  	MSDOS_SUPPORT \
  	WINNT_SUPPORT \
***************
*** 870,875 ****
--- 871,877 ----
  	../lisp/emacs-lisp/timer.elc \
  	../lisp/vc-hooks.elc \
  	../lisp/ediff-hook.elc \
+ 	../lisp/rfn-eshadow.elc \
  	../lisp/widget.elc \
  	../lisp/window.elc \
  	../lisp/version.el
============================================================

Re: file-name-shadow-mode

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> The following four patches enable `file-name-shadow-mode' by default,
> preload rfn-eshadow.el and make define-minor-mode accept an
> :initialize argument.  (It currently kind of accepts such an argument,
> but the result is that the macroexpanded defcustom contains two
> :initialize keywords, a bug.)

I don't see where the bug is: the way custom-declare-variable is
defined, the last such keyword will get used, and that's that.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Stefan Monnier]

> :initialize keywords, a bug.)  To make things work correctly, I had to
> rearrange define-minor-mode to define the mode function before the
> defcustom.

I don't think this is a good idea.

> Apparently, this gives no compiler warnings (I checked), even though the
> function uses the mode variable, which is, in the new version, not
> yet defcustomed.

I'd be surprised if it doesn't introduce spurious warnings at
various places.


        Stefan

Re: file-name-shadow-mode

[Luc Teirlinck]

Stefan Monnier wrote:

   > Apparently, this gives no compiler warnings (I checked), even though the
   > function uses the mode variable, which is, in the new version, not
   > yet defcustomed.

   I'd be surprised if it doesn't introduce spurious warnings at
   various places.

I indeed did not look closely enough (it did not produce warnings for
file-name-shadow-mode during bootstrapping).  This problem could be
easily circumvented by a compiler defvar for the mode variable at the
beginning of the define-minor-mode expansion.  Otherwise, setting
:initialize to something else but `custom-initialize-default' is
probably not going to work for minor modes defined with
`define-minor-mode'.  (It definitely does not work with the current
code.)  One would then have to enable such minor modes in startup.el.
The best way to do that would probably be with
`custom-reevaluate-setting', since that way any changes to the
defcustom automatically take effect in startup.el too.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Stefan Monnier]

> beginning of the define-minor-mode expansion.  Otherwise, setting
> :initialize to something else but `custom-initialize-default' is
> probably not going to work for minor modes defined with
> `define-minor-mode'.  (It definitely does not work with the current
> code.)

I think it's OK that way.  Last time we had a discussion around this, it
became clear to me that the :init value should *describe* the state you
get when loading the file, rather than *decide* it.

This is because if the minor mode's code needs to be run to properly set the
value, you can't run it safely before the whole file is loaded (since the
minor mode's code might call functions defined further down in the file).

So I think it's best to keep the :init value at nil and then elsewhere
do a customize-set-variable (in startup.el, in loadup.el, or even at the
end of rfn-shadow.el).  That's the cleaner solution.


        Stefan

Re: file-name-shadow-mode

[Luc Teirlinck]

Stefan Monnier wrote:

   This is because if the minor mode's code needs to be run to properly set the
   value, you can't run it safely before the whole file is loaded (since the
   minor mode's code might call functions defined further down in the file).

That is not really different from any other defcustom with a :set
function:  you have to define all functions used by the :set function
before the defcustom.  Sometimes these functions themselves refer to
the defcustomed variable.  Then you have to do:

(defvar var)
defun's
(defcustom var...

to avoid compiler warnings.

Note that if you do (defvar foo (bar ...)), then bar has to be defined
before foo too.  I do not see where the problem is.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Luc Teirlinck]

SDtefan Monnier wrote:

   So I think it's best to keep the :init value at nil

_Only_ if one would decide to keep the default value nil too. in which
case there is no problem.  The value given in the defcustom *needs* to
be the real default value.  Otherwise "Erase Customization" in Custom
produces a bug.

   and then elsewhere do a customize-set-variable (in startup.el, in
   loadup.el, or even at the end of rfn-shadow.el).

Definitely not in rfn-eshadow.el, because then reloading the file
_would_ have an effect.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Luc Teirlinck]

I am no longer sure that enabling `file-name-shadow-mode' by default
is really a good idea.  (_Not_ because it would be difficult: it could
easily be done using `custom-reevaluate-setting' in startup.el, even
without any changes to define-minor-mode.)

The problem is a comment in rfn-eshadow.el:

  ;; This is not really a correct implementation; it won't always do the
  ;; right thing in the presence of environment variables that
  ;; substitute-in-file-name would expand; currently it just assumes any
  ;; environment variable contains an absolute filename.

In other words, rfn-eshadow.el assumes that the value of _every_
environment variable is an absolute file name and it yields very
confusing results when other environment variables are used in file
names, say "/home/$USER".

Sincerely,

Luc.

Re: file-name-shadow-mode

[Miles Bader]

On Sat, 19 Mar 2005 20:30:57 -0600 (CST), Luc Teirlinck
<teirllm@dms.auburn.edu> wrote:
> In other words, rfn-eshadow.el assumes that the value of _every_
> environment variable is an absolute file name and it yields very
> confusing results when other environment variables are used in file
> names, say "/home/$USER".

Well I'm obviously biased, but:

I doubt 99% of the people out there use environment variables while
typing filenames _at all_.  However I'm probably one of the few that
does -- in fact I use them heavily -- and I've _never_ wanted to use a
relative filename from one.

The functionality of rfn-eshadow is quite useful to the vast majority
of users, and indeed is much-requested, and you want to not enable it
because you found a minor flaw?  A flaw whose only effect is to make
the prompt slightly less clear that it could be?  I expect that the
traditional "double slash" behavior of Emacs filename input -- which
is what rfn-eshadow fixes -- confuses far, _far_, more people.

[I think it's probably possible to fix this -- e.g., by generating a
regexp of all non-absolute environment variables and glomming it onto
the rexexp used for filenames.  But my basic point is that it scarcely
matters.]

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: file-name-shadow-mode

[David Kastrup]

Miles Bader <snogglethorpe@gmail.com> writes:

> On Sat, 19 Mar 2005 20:30:57 -0600 (CST), Luc Teirlinck
> <teirllm@dms.auburn.edu> wrote:
>> In other words, rfn-eshadow.el assumes that the value of _every_
>> environment variable is an absolute file name and it yields very
>> confusing results when other environment variables are used in file
>> names, say "/home/$USER".
>
> Well I'm obviously biased, but:
>
> I doubt 99% of the people out there use environment variables while
> typing filenames _at all_.  However I'm probably one of the few that
> does -- in fact I use them heavily -- and I've _never_ wanted to use a
> relative filename from one.
>
> The functionality of rfn-eshadow is quite useful to the vast majority
> of users, and indeed is much-requested, and you want to not enable it
> because you found a minor flaw?  A flaw whose only effect is to make
> the prompt slightly less clear that it could be?  I expect that the
> traditional "double slash" behavior of Emacs filename input -- which
> is what rfn-eshadow fixes -- confuses far, _far_, more people.
>
> [I think it's probably possible to fix this -- e.g., by generating a
> regexp of all non-absolute environment variables and glomming it onto
> the rexexp used for filenames.  But my basic point is that it scarcely
> matters.]

If anyone should by chance encounter it, I'd almost be willing to bet
he'll shrug it off as "peculiarity, to be expected with that kind of
abuse".  I doubt we would get a bug report for it from anyone...

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Luc Teirlinck]

Miles Bader wrote:

   A flaw whose only effect is to make the prompt slightly less clear
   that it could be?

A flaw that might give people the false impression that `$' after /
has the same effect as / or ~ after /.

   I expect that the
   traditional "double slash" behavior of Emacs filename input -- which
   is what rfn-eshadow fixes -- confuses far, _far_, more people.

I do not understand.  It confuses people who want to use empty file
or directory names?  I doudt that is going to work anyway.

Sincerely,

Luc.

Re: file-name-shadow-mode

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Miles Bader wrote:
>
>    A flaw whose only effect is to make the prompt slightly less clear
>    that it could be?
>
> A flaw that might give people the false impression that `$' after /
> has the same effect as / or ~ after /.
>
>    I expect that the
>    traditional "double slash" behavior of Emacs filename input -- which
>    is what rfn-eshadow fixes -- confuses far, _far_, more people.
>
> I do not understand.  It confuses people who want to use empty file
> or directory names?  I doudt that is going to work anyway.

Posix specifies that multiple slashes are the same as single slashes
in Shells and system calls.  So Emacs' behavior is surprising, though
useful.

Luc, you have recently acquired a tendency to veto everything on minor
technicalities without weighing the advantages against them.

While it certainly is always worthwhile to aim for the best possible
solution, foregoing any improvement because it is not yet absolutely
perfect is contraproductive.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Richard Stallman]

    A flaw that might give people the false impression that `$' after /
    has the same effect as / or ~ after /.

That's a problem, but not a very big one.  This misunderstanding would
be minro, not fatal, and it would happen rarely.  And people can still
read the manual and find out the full facts.

All kinds of incorrect user behavior in Emacs are not equally bad.
But that is not generally true.  Incorrect behavior is not necessarily
a disaster, it is merly more or less undedirable.  One has to try to
judge problems by their importance.

Re: file-name-shadow-mode

[Richard Stallman]

    The functionality of rfn-eshadow is quite useful to the vast majority
    of users, and indeed is much-requested, and you want to not enable it
    because you found a minor flaw?  A flaw whose only effect is to make
    the prompt slightly less clear that it could be?

I agree: this bug is not enough reason not to enable the feature.
However, it would be nice to fix the bug.

    [I think it's probably possible to fix this -- e.g., by generating a
    regexp of all non-absolute environment variables and glomming it onto
    the rexexp used for filenames.  But my basic point is that it scarcely
    matters.]

Could you try fixing it that way?

Re: file-name-shadow-mode

[Luc Teirlinck]

Richard Stallman wrote:

       [I think it's probably possible to fix this -- e.g., by generating a
       regexp of all non-absolute environment variables and glomming it onto
       the rexexp used for filenames.  But my basic point is that it scarcely
       matters.]

   Could you try fixing it that way?

It would probably be a lot better to do the complement: look for all
environment variables whose value starts with / or ~.  Emacs supports
file names that start with $ if the $ is not followed by an
environment variable, or if the $ is doubled up.  I may be overlooking
something obvious, but is therE a reason not to use `getenv'?

Sincerely,

Luc.

Re: file-name-shadow-mode

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Richard Stallman wrote:
>
>        [I think it's probably possible to fix this -- e.g., by generating a
>        regexp of all non-absolute environment variables and glomming it onto
>        the rexexp used for filenames.  But my basic point is that it scarcely
>        matters.]
>
>    Could you try fixing it that way?
>
> It would probably be a lot better to do the complement: look for all
> environment variables whose value starts with / or ~.  Emacs
> supports file names that start with $ if the $ is not followed by an
> environment variable, or if the $ is doubled up.  I may be
> overlooking something obvious, but is therE a reason not to use
> `getenv'?

The more robust solution would probably be to see what happens when
using substitute-in-file-name on the substring starting with $, and
see whether the result is an absolute file name.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Luc Teirlinck]

file-name-shadow-mode should also look for /: at the beginning of a
file name and leave the file name completely alone if it starts with /:

Currently, erroneous highlighting occurs after things like /:/~ and /:/$

Sincerely,

Luc.

Re: file-name-shadow-mode

[Luc Teirlinck]

>From my previous message:

   file-name-shadow-mode should also look for /: at the beginning of a
   file name and leave the file name completely alone if it starts with /:

   Currently, erroneous highlighting occurs after things like /:/~ and /:/$

As well as after things like /:~//info.  After /:, Emacs replaces
consecutive multiple slashes with a single slash.  Hence, /:~//info
refers to ~/info and not to /info, as file-name-shadow-mode's
highlighting mistakenly suggests.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Stefan Monnier]

> As well as after things like /:~//info.  After /:, Emacs replaces

BTW, while testing my code, I was actually surprised (although it makes
perfect sense) by the fact that "C-x C-f /:~//info" doesn't behave as
explained above.  You need to do "C-x C-f C-a C-k /:~//info".


        Stefan

Re: file-name-shadow-mode

[Luc Teirlinck]

Stefan Monnier wrote:

   BTW, while testing my code, I was actually surprised (although it makes
   perfect sense) by the fact that "C-x C-f /:~//info" doesn't behave as
   explained above.  You need to do "C-x C-f C-a C-k /:~//info".

The "/:" needs to be at the very beginning of the minibuffer contents
to work.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Stefan Monnier]

>    BTW, while testing my code, I was actually surprised (although it makes
>    perfect sense) by the fact that "C-x C-f /:~//info" doesn't behave as
>    explained above.  You need to do "C-x C-f C-a C-k /:~//info".

> The "/:" needs to be at the very beginning of the minibuffer contents
> to work.

That was my point: this is surprising.

My intuitive understanding of the behavior of substitute-in-file-name is
that if <bar> is an absolute file name, then <foo>/<bar> is treated just
like <bar> (provided <foo> is a "normal" file name, not one that matches
/: or some other magic file name handler).

But /home//:~//info will *not* be treated like /:~//info.
Whether it's a bug or feature, I don't know, but I find it surprising.


        Stefan

Re: file-name-shadow-mode

[Richard Stallman]

    But /home//:~//info will *not* be treated like /:~//info.
    Whether it's a bug or feature, I don't know, but I find it surprising.

I agree it would be more natural to change this.
Just because /: suppresses special treatment of following text
does not mean previous text must suppress special treatment of /:.

It has some chance of breaking something, but I think 
that is not likely to happen.

Re: file-name-shadow-mode

[Stefan Monnier]

>     But /home//:~//info will *not* be treated like /:~//info.
>     Whether it's a bug or feature, I don't know, but I find it surprising.

> I agree it would be more natural to change this.
> Just because /: suppresses special treatment of following text
> does not mean previous text must suppress special treatment of /:.

I've just installed a change that fixes the above.


        Stefan

Re: file-name-shadow-mode

[Stefan Monnier]

>     The functionality of rfn-eshadow is quite useful to the vast majority
>     of users, and indeed is much-requested, and you want to not enable it
>     because you found a minor flaw?  A flaw whose only effect is to make
>     the prompt slightly less clear that it could be?

I agree that the approximate handling of envvars never seriously bothered
me, but it's clearly a bug we should fix.
I've OTOH been annoyed by the bad interaction with url-handler-mode, where
the file name is most naturally something of the form "http://foo/bar" and
rfn-eshadow.el systematically hides the "http:/".

> I agree: this bug is not enough reason not to enable the feature.
> However, it would be nice to fix the bug.

>     [I think it's probably possible to fix this -- e.g., by generating a
>     regexp of all non-absolute environment variables and glomming it onto
>     the rexexp used for filenames.  But my basic point is that it scarcely
>     matters.]

> Could you try fixing it that way?

How 'bout the patch below which makes no assumption (that I know of) about
substitute-in-file-name, and will thus correctly even with weird
magic file name handlers.


        Stefan


--- rfn-eshadow.el	01 sep 2003 11:45:14 -0400	1.12
+++ rfn-eshadow.el	21 mar 2005 08:19:24 -0500	
@@ -1,6 +1,6 @@
 ;;; rfn-eshadow.el --- Highlight `shadowed' part of read-file-name input text
 ;;
-;; Copyright (C) 2000, 2001, 2002 Free Software Foundation, Inc.
+;; Copyright (C) 2000, 2001, 2002, 2005  Free Software Foundation, Inc.
 ;;
 ;; Author: Miles Bader <miles@gnu.org>
 ;; Keywords: convenience minibuffer
@@ -176,25 +176,25 @@
 This is intended to be used as a minibuffer post-command-hook for
 `file-name-shadow-mode'; the minibuffer should have already
 been set up by `rfn-eshadow-setup-minibuffer'."
-  ;; This is not really a correct implementation; it won't always do the
-  ;; right thing in the presence of environment variables that
-  ;; substitute-in-file-name would expand; currently it just assumes any
-  ;; environment variable contains an absolute filename.
-  (save-excursion
-    (let ((inhibit-point-motion-hooks t))
-      (goto-char (minibuffer-prompt-end))
-      ;; Update the overlay (which will evaporate if it's empty).
-      (move-overlay rfn-eshadow-overlay
-		    (point)
-		    (if (looking-at rfn-eshadow-regexp)
-			(match-end 1)
-		      (point))))))
-
-\f
-;;; Note this definition must be at the end of the file, because
-;;; `define-minor-mode' actually calls the mode-function if the
-;;; associated variable is non-nil, which requires that all needed
-;;; functions be already defined.  [This is arguably a bug in d-m-m]
+  (condition-case nil
+      (let ((goal (substitute-in-file-name (minibuffer-contents)))
+	    (start (minibuffer-prompt-end))
+	    (end (point-max)))
+	;; Binary search for the greatest position still equivalent to
+	;; the whole.
+	(while (< (1+ start) end)
+	  (let ((mid (/ (+ start end) 2)))
+	    (if (equal (condition-case nil
+			   (substitute-in-file-name
+			    (buffer-substring mid (point-max)))
+			 (error nil))
+		       goal)
+		(setq start mid)
+	      (setq end mid))))
+	(move-overlay rfn-eshadow-overlay (minibuffer-prompt-end) start))
+    ;; `substitute-in-file-name' can fail on partial input.
+    (error nil)))
+\f
 ;;;###autoload
 (define-minor-mode file-name-shadow-mode
   "Toggle File-Name Shadow mode.
@@ -222,5 +222,5 @@
 
 (provide 'rfn-eshadow)
 
-;;; arch-tag: dcf70a52-0115-4ec2-b1e3-4f8d3541a888
+;; arch-tag: dcf70a52-0115-4ec2-b1e3-4f8d3541a888
 ;;; rfn-eshadow.el ends here

Re: file-name-shadow-mode

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> How 'bout the patch below which makes no assumption (that I know of)
> about substitute-in-file-name, and will thus correctly even with
> weird magic file name handlers.

> +  (condition-case nil
> +      (let ((goal (substitute-in-file-name (minibuffer-contents)))
> +	    (start (minibuffer-prompt-end))
> +	    (end (point-max)))
> +	;; Binary search for the greatest position still equivalent to
> +	;; the whole.
> +	(while (< (1+ start) end)
> +	  (let ((mid (/ (+ start end) 2)))
> +	    (if (equal (condition-case nil
> +			   (substitute-in-file-name
> +			    (buffer-substring mid (point-max)))
> +			 (error nil))
> +		       goal)
> +		(setq start mid)
> +	      (setq end mid))))
> +	(move-overlay rfn-eshadow-overlay (minibuffer-prompt-end) start))
> +    ;; `substitute-in-file-name' can fail on partial input.
> +    (error nil)))

Cute.  It makes the assumption, however, that there is a unique point
where that equivalence occurs.  I don't think that this is correct:
whenever you split in the middle of an environment variable name,
you'll get a nonmatch that can turn into a match if you happen to look
further.  So it would appear to me that one does not get around either
recognizing syntactical entities and skipping them completely instead
of partially, or going through the whole expression anyhow.

It would appear to me that by far the most robust and also efficient
approach would probably be to provide a
substitute-in-file-name-partially primitive that would expand
everything until the next single non-literal change, returning the
relevant information for splicing in the change and continuing the
scan.  Possibly it might come handy for other file name manipulation
tasks.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Stefan Monnier]

> Cute.  It makes the assumption, however, that there is a unique point
> where that equivalence occurs.

Not quite, it makes the assumption that if

   (substitute-in-file-name (buffer-substring N END))
   == (substitute-in-file-name (buffer-substring START END))

and (< START N) then

   (substitute-in-file-name (buffer-substring (1- N) END))
   == (substitute-in-file-name (buffer-substring START END))

This assumption is inded not always verified.  But the only counter-example
I can think of is with the "/:" oddity that I mentioned in another message
(a similar one occurs with http://).  I was thinking of adding a check to
eliminate those very special cases.

In any case, the good side of my patch is that even if doesn't do quite what
you want, it guarantees that `substitute-in-file-name' returns the same
thing when passed the whole minibuffer's content or just the
non-shadowed part.  I.e. it never gives a *wrong* result.

> I don't think that this is correct: whenever you split in the middle of an
> environment variable name, you'll get a nonmatch that can turn into
> a match if you happen to look further.

Hmmm.... could you explain what concrete case you have in mind?


        Stefan

Re: file-name-shadow-mode

[Richard Stallman]

    Cute.  It makes the assumption, however, that there is a unique point
    where that equivalence occurs.  I don't think that this is correct:
    whenever you split in the middle of an environment variable name,
    you'll get a nonmatch that can turn into a match if you happen to look
    further.

What about doing a linear scan from the end?  It might be too
slow...but it might be perfectly fast enough.  It is sufficient
to test places where there is a /, ~, or $.  There usually
won't be many of them.

Re: file-name-shadow-mode

[Miles Bader]

On Mon, 21 Mar 2005 08:24:38 -0500, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> How 'bout the patch below which makes no assumption (that I know of) about
> substitute-in-file-name, and will thus correctly even with weird
> magic file name handlers.

Um, I should note that the original code was designed to be fast and
not cons unnecessarily, because it runs on every keystroke -- that's
the reason it uses just a regexp match and a move-overlay.  Your
suggested replacement looks ... less careful.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: file-name-shadow-mode

[David Kastrup]

Miles Bader <snogglethorpe@gmail.com> writes:

> On Mon, 21 Mar 2005 08:24:38 -0500, Stefan Monnier
> <monnier@iro.umontreal.ca> wrote:
>> How 'bout the patch below which makes no assumption (that I know of) about
>> substitute-in-file-name, and will thus correctly even with weird
>> magic file name handlers.
>
> Um, I should note that the original code was designed to be fast and
> not cons unnecessarily, because it runs on every keystroke -- that's
> the reason it uses just a regexp match and a move-overlay.  Your
> suggested replacement looks ... less careful.

substitute-in-file-name is a built-in function in `C source code'.
(substitute-in-file-name FILENAME &optional PARSEINFO)

Substitute environment variables referred to in FILENAME.
`$FOO' where FOO is an environment variable name means to substitute
the value of that variable.  The variable name should be terminated
with a character not a letter, digit or underscore; otherwise, enclose
the entire variable name in braces.
If `/~' appears, all of FILENAME through that `/' is discarded.

On VMS, `$' substitution is not done; this function does little and only
duplicates what `expand-file-name' does.

If PARSEINFO is not NIL, it is a list used or reused for storing
parsing information (it is extended as necessary).

Elements 2N point to positions in the original string, Elements 2N+1
to corresponding positions in the substituted string.

[...]

Something like that.  And then one looks for the last [x,0] pair in
the list, and x is the position from which to shadow.

No consing done except when the syntax changes on entry.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[David Kastrup]

David Kastrup <dak@gnu.org> writes:

> Miles Bader <snogglethorpe@gmail.com> writes:
>
>> On Mon, 21 Mar 2005 08:24:38 -0500, Stefan Monnier
>> <monnier@iro.umontreal.ca> wrote:
>>> How 'bout the patch below which makes no assumption (that I know of) about
>>> substitute-in-file-name, and will thus correctly even with weird
>>> magic file name handlers.
>>
>> Um, I should note that the original code was designed to be fast and
>> not cons unnecessarily, because it runs on every keystroke -- that's
>> the reason it uses just a regexp match and a move-overlay.  Your
>> suggested replacement looks ... less careful.
>
> substitute-in-file-name is a built-in function in `C source code'.
> (substitute-in-file-name FILENAME &optional PARSEINFO)
>
> Substitute environment variables referred to in FILENAME.
> `$FOO' where FOO is an environment variable name means to substitute
> the value of that variable.  The variable name should be terminated
> with a character not a letter, digit or underscore; otherwise, enclose
> the entire variable name in braces.
> If `/~' appears, all of FILENAME through that `/' is discarded.
>
> On VMS, `$' substitution is not done; this function does little and only
> duplicates what `expand-file-name' does.
>
> If PARSEINFO is not NIL, it is a list used or reused for storing
> parsing information (it is extended as necessary).
>
> Elements 2N point to positions in the original string, Elements 2N+1
> to corresponding positions in the substituted string.

Add to that: if the source element does not have a corresponding
position in the output (i.e., because a later element reintroduces an
absolute path element), the corresponding element 2N+1 is nil.

> [...]

Now we can just do

(let ((shadowlist (list nil)) p shadowpos)

   [start some input loop, inside have:]

   (substitute-in-filename filename shadowlist)
   (setq p shadowlist shadowpos 0)
   (while (setq p (memq nil p))
     (setq shadowpos (car (setq p (cdr p)))))

> No consing done except when the syntax changes on entry.

This holds the same.  Is the proposed data structure unnecessary
complicated (as in "nobody will ever need that") or too simple (as in
"it requires extra work to check that a given source piece is verbatim
in the output)?

Should there be a possibility to actually reuse already scanned
strings in case that only stuff gets appended at the end?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Stefan Monnier]

>> How 'bout the patch below which makes no assumption (that I know of) about
>> substitute-in-file-name, and will thus correctly even with weird
>> magic file name handlers.

> Um, I should note that the original code was designed to be fast and
> not cons unnecessarily, because it runs on every keystroke -- that's
> the reason it uses just a regexp match and a move-overlay.  Your
> suggested replacement looks ... less careful.

I'm fully aware of it, of course.  Given the binary search, it should around
the loop about 5-7 times, and substitute-in-file-name shouldn't do that much
consing itself, so I figured it was worth trying.

Also we could easily optimize the common case (where the overlay is not
moved) using a total of 3 calls to substitute-in-file-name in the
common case.

But given the fact that icomplete.el has been usable for a long time
already, I suspect that the performance issue is only in our heads,


        Stefan

Re: file-name-shadow-mode

[Luc Teirlinck]

Stefan Monnier wrote:

   How 'bout the patch below which makes no assumption (that I know of) about
   substitute-in-file-name, and will thus correctly even with weird
   magic file name handlers.

Seems to work very well.

   But given the fact that icomplete.el has been usable for a long time
   already, I suspect that the performance issue is only in our heads,

That is what we have to assume unless somebody would come up with some
really strong evidence to the contrary.   On my machine, the
highlighting corrects instantaneously, even for complex examples that
completely change by deletion or addition of a single character.

Sincerely,

Luc.

Re: file-name-shadow-mode

[Stefan Monnier]

> How 'bout the patch below which makes no assumption (that I know of) about
> substitute-in-file-name, and will thus work correctly even with weird
> magic file name handlers.

Regarding performance of my code: I just bumped into a performance problem.
The problem is that substitute-in-file-name can take a non-negligible amount
of time to execute when there's a "~user" in the file name, because it calls
getpwnam to figure out whether "user" actually exists or not.

At least here with 8K users in our YP database, my code causes
file-name-shadow-mode to take around 0.5-1s to refresh the screen after each
key stroke if there's a ~user in the file name I'm editing.

So I've tweaked my code to streamline the common case where the shadow
doesn't need to be moved.  I also fixed the problem where a file name like
"/home/foo/bar/:toto//tata" was sometimes shadowed as "r/:toto//tata"
instead of just "/tata".


        Stefan


--- orig/lisp/rfn-eshadow.el
+++ mod/lisp/rfn-eshadow.el
@@ -97,7 +97,7 @@
   '(face file-name-shadow field shadow)
   "Properties given to the `shadowed' part of a filename in the minibuffer.
 Only used when `file-name-shadow-mode' is active.
-If emacs is not running under a window system,
+If Emacs is not running under a window system,
 `file-name-shadow-tty-properties' is used instead."
   :type file-name-shadow-properties-custom-type
   :group 'minibuffer)
@@ -123,20 +123,6 @@
 \f
 ;;; Internal variables
 
-;; Regexp to locate dividing point between shadow and real pathname
-(defconst rfn-eshadow-regexp
-  (cond ((memq system-type '(ms-dos windows-nt))
-	 ;; This horrible regexp considers the following patterns as
-	 ;; starting an absolute pathname, when following a `/' or an `\':
-	 ;;   L:  /  //  ~  $  \\  \\\\
-	 "\\(.*[^/]+/+?\\|/*?\\|\\)\\(~\\|$[^$]\\|$\\'\\|[][\\^a-z]:\\|//?\\([^][\\^a-z/$~]\\|[^/$~][^:]\\|[^/$~]?\\'\\)\\)")
-	(t
-	 ;; default is for unix-style filenames
-	 "\\(.*/\\)\\([/~]\\|$[^$]\\|$\\'\\)"))
-  "Regular expression used to match shadowed filenames.
-There should be at least one regexp group; the end of the first one
-is used as the end of the shadowed portion of the filename.")
-
 ;; A list of minibuffers to which we've added a post-command-hook.
 (defvar rfn-eshadow-frobbed-minibufs nil)
 
@@ -170,31 +156,54 @@
     (add-to-list 'rfn-eshadow-frobbed-minibufs (current-buffer))
     (add-hook 'post-command-hook #'rfn-eshadow-update-overlay nil t)))
 
+(defsubst rfn-eshadow-sifn-equal (goal pos)
+  (equal goal (buffer-substring-no-properties pos (point-max))))
+
 ;; post-command-hook to update overlay
 (defun rfn-eshadow-update-overlay ()
   "Update `rfn-eshadow-overlay' to cover shadowed part of minibuffer input.
 This is intended to be used as a minibuffer post-command-hook for
 `file-name-shadow-mode'; the minibuffer should have already
 been set up by `rfn-eshadow-setup-minibuffer'."
-  ;; This is not really a correct implementation; it won't always do the
-  ;; right thing in the presence of environment variables that
-  ;; substitute-in-file-name would expand; currently it just assumes any
-  ;; environment variable contains an absolute filename.
-  (save-excursion
-    (let ((inhibit-point-motion-hooks t))
-      (goto-char (minibuffer-prompt-end))
-      ;; Update the overlay (which will evaporate if it's empty).
-      (move-overlay rfn-eshadow-overlay
-		    (point)
-		    (if (looking-at rfn-eshadow-regexp)
-			(match-end 1)
-		      (point))))))
+  ;; This code usually is instantaneous, but if the file name includes
+  ;; a "~<user>", substitute-in-file-name will lookup your system's list of
+  ;; users to see whether "<user>" is an actual user or not, and that
+  ;; can sometimes take a while, so we wrap this in `while-no-input'.
+  (while-no-input
+    (condition-case nil
+	(let ((goal (substitute-in-file-name (minibuffer-contents)))
+	      (mid (overlay-end rfn-eshadow-overlay))
+	      (start (minibuffer-prompt-end))
+	      (end (point-max)))
+	  (unless
+	      ;; Catch the common case where the shadow does not need to move.
+	      (and mid
+		   (or (eq mid end)
+		       (not (rfn-eshadow-sifn-equal goal (1+ mid))))
+		   (or (eq mid start)
+		       (rfn-eshadow-sifn-equal goal mid)))
+	    ;; Binary search for the greatest position still equivalent to
+	    ;; the whole.
+	    (while (or (< (1+ start) end)
+		       (if (and (< (1+ end) (point-max))
+				(rfn-eshadow-sifn-equal goal (1+ end)))
+			   ;; (SIFN end) != goal, but (SIFN (1+end)) == goal,
+			   ;; We've reached a discontinuity: this can happen
+			   ;; e.g. if `end' point to "/:...".
+			   (setq start (1+ end) end (point-max))))
+	      (setq mid (/ (+ start end) 2))
+	      (if (equal (condition-case nil
+			     (rfn-eshadow-sifn-equal goal mid)
+			   (error nil))
+			 goal)
+		  (setq start mid)
+		(setq end mid)))
+	    (move-overlay rfn-eshadow-overlay (minibuffer-prompt-end) start)))
+      ;; `substitute-in-file-name' can fail on partial input.
+      ;; (error nil)
+      )))
 
 \f
-;;; Note this definition must be at the end of the file, because
-;;; `define-minor-mode' actually calls the mode-function if the
-;;; associated variable is non-nil, which requires that all needed
-;;; functions be already defined.  [This is arguably a bug in d-m-m]
 ;;;###autoload
 (define-minor-mode file-name-shadow-mode
   "Toggle File-Name Shadow mode.
@@ -222,5 +231,5 @@
 
 (provide 'rfn-eshadow)
 
-;;; arch-tag: dcf70a52-0115-4ec2-b1e3-4f8d3541a888
+;; arch-tag: dcf70a52-0115-4ec2-b1e3-4f8d3541a888
 ;;; rfn-eshadow.el ends here

Re: file-name-shadow-mode

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> How 'bout the patch below which makes no assumption (that I know of) about
>> substitute-in-file-name, and will thus work correctly even with weird
>> magic file name handlers.
>
> Regarding performance of my code: I just bumped into a performance
> problem.  The problem is that substitute-in-file-name can take a
> non-negligible amount of time to execute when there's a "~user" in
> the file name, because it calls getpwnam to figure out whether
> "user" actually exists or not.
>
> At least here with 8K users in our YP database, my code causes
> file-name-shadow-mode to take around 0.5-1s to refresh the screen
> after each key stroke if there's a ~user in the file name I'm
> editing.

You have seen Richard's proposal of going backwards linearly from the
end and only checking at the "critical" characters like ~, / and $?
It should also be possible to cache a piece of the last shadowing
action and don't look again if no "critical" character has been added
after the previous one in the nonshadowed section.

But actually, I think my proposal about a C level interface into
substitute-in-file-name would be most efficient, robust and
unproblematic in the long run, and possibly useful for other
applications as well.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Richard Stallman]

    But actually, I think my proposal about a C level interface into
    substitute-in-file-name would be most efficient, robust and
    unproblematic in the long run, and possibly useful for other
    applications as well.

I don't want to do this, because it puts too much complexity in a
place where it could cause trouble.  I'd rather just stay away from it.

To duplicate just the parsing aspect of substitute-in-file-name
for this one purpose would be better.

Re: file-name-shadow-mode

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     But actually, I think my proposal about a C level interface into
>     substitute-in-file-name would be most efficient, robust and
>     unproblematic in the long run, and possibly useful for other
>     applications as well.
>
> I don't want to do this, because it puts too much complexity in a
> place where it could cause trouble.  I'd rather just stay away from
> it.
>
> To duplicate just the parsing aspect of substitute-in-file-name
> for this one purpose would be better.

My opinion about that differs; but if I find time arguing that point,
I'll do it with a patch.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: file-name-shadow-mode

[Richard Stallman]

    Hi, if I remember correctly, we had agreed a long time ago to make
    file-name-shadow-mode active by default for the release.

What I said is that I would try it myself.  I have done so.  It seems
useful and painless, so now I agree it should be the default.

Could you enable it?