Re: tiny patch to ange-ftp

Re: tiny patch to ange-ftp

[Stefan Monnier]

> Now I'm able to do things by setting `ange-ftp-send-hash' as nil
> and modifying `ange-ftp-insert-directory' slightly.  However,
> I'm not satisfied with it.  Though I'm making an effort at
> another work now, I'm going to tackle it again in the near
> future.  It will be completed if the problem of which hash mark
> printing breaks the synchronous operation is solved.

> I'm currently using ange-ftp.el which corrected as follows:

Your patch looks like a good first step.  I would check `full'
rather than (file-name-directory file) in order to determine whether to do
a full dir listing or to extract a single line.

Also I hoped that we would be able to use the cache so as to avoid doing the
full-listing+extract-line thingy repeatedly.  Using ange-ftp-get-files
should do the trick here.  But that can be changed later on.

Given that this fixes the problem mentioned in ange-ftp-send-cmd and should
be orthogonal to the problem of hash marks, I suggest we install your patch.

Any objections?


        Stefan


> *** ange-ftp.el~	Thu Aug 11 21:55:21 2005
> --- ange-ftp.el	Thu Sep  1 23:08:06 2005
> ***************
> *** 4498,4504 ****
>   	   (if wildcard
>   	       (let ((default-directory (file-name-directory file)))
>   		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
> ! 	     (ange-ftp-ls file switches full))))
>         (ange-ftp-real-insert-directory file switches wildcard full))))
  
>   (defun ange-ftp-dired-uncache (dir)
> --- 4498,4519 ----
>   	   (if wildcard
>   	       (let ((default-directory (file-name-directory file)))
>   		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
> ! 	     ;; That `file' doesn't have directory components means we're
> ! 	     ;; going to do `ls' for a single file in the current directory.
> ! 	     (if (file-name-directory file)
> ! 		 (ange-ftp-ls file switches full)
> ! 	       ;; Get a full listing of the current directory and extract
> ! 	       ;; the line corresponding to `file'.
> ! 	       (when (string-match "d\\'" switches)
> ! 		 ;; Remove "d" which dired added to `switches'.
> ! 		 (setq switches (substring switches 0 (match-beginning 0))))
> ! 	       (setq tem (ange-ftp-ls "." switches full))
> ! 	       (let ((case-fold-search nil))
> ! 		 ;; Assume the file name which a single space precedes
> ! 		 ;; appears at the end of a line.
> ! 		 (if (string-match (concat "^.+ " (regexp-quote file) "$") tem)
> ! 		     (concat (match-string 0 tem) "\n")
> ! 		   ""))))))
>         (ange-ftp-real-insert-directory file switches wildcard full))))
  
>   (defun ange-ftp-dired-uncache (dir)

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

Sorry for the late reply.  I seem to have fixed all the problems
that I grasp.

>>>>> In <jwvwtlzimzg.fsf-monnier+emacs@gnu.org> Stefan Monnier wrote:

> Your patch looks like a good first step.  I would check `full'
> rather than (file-name-directory file) in order to determine whether to do
> a full dir listing or to extract a single line.

I did so.

> Also I hoped that we would be able to use the cache so as to avoid doing the
> full-listing+extract-line thingy repeatedly.  Using ange-ftp-get-files
> should do the trick here.  But that can be changed later on.

Well, using the cache seems useless for the remote file which is
newly uploaded. Am I wrong?  I tried some ways but I didn't get
the right way to use those functions so far.

> Given that this fixes the problem mentioned in ange-ftp-send-cmd and should
> be orthogonal to the problem of hash marks, I suggest we install your patch.

> Any objections?

I noticed `ange-ftp-process-filter' does not extract all hash
marks because of the too strict regexp and it breaks the
synchronous operation while at least uploading files.

Here's a new patch:

2005-09-08  Katsumi Yamaoka  <yamaoka@jpl.org>  (tiny change)

	* net/ange-ftp.el (ange-ftp-process-filter): Fix regexp to extract
	hash marks.
	(ange-ftp-insert-directory): Work with a single file listing.

--8<---------------cut here---------------start------------->8---
*** ange-ftp.el~	Thu Aug 11 21:55:21 2005
--- ange-ftp.el	Thu Sep  8 03:57:18 2005
***************
*** 1656,1662 ****
  
  	      ;; handle hash mark printing
  	      (and ange-ftp-process-busy
! 		   (string-match "\\`#+\\'" str)
  		   (setq str (ange-ftp-process-handle-hash str)))
  	      (comint-output-filter proc str)
  	      ;; Replace STR by the result of the comint processing.
--- 1656,1662 ----
  
  	      ;; handle hash mark printing
  	      (and ange-ftp-process-busy
! 		   (string-match "#+" str)
  		   (setq str (ange-ftp-process-handle-hash str)))
  	      (comint-output-filter proc str)
  	      ;; Replace STR by the result of the comint processing.
***************
*** 2332,2347 ****
  	;; This works around a misfeature of some versions of netbsd ftpd
  	;; where `ls' can only take one argument: either one set of flags
  	;; or a file/directory name.
- 	;; FIXME: if we're trying to `ls' a single file, this fails since we
- 	;; can't cd to a file.  We can't fix this problem here, tho, because
- 	;; at this point we don't know whether the argument is a file or
- 	;; a directory.  Such an `ls' is only every used (apparently) from
- 	;; `insert-directory' when the `full-directory-p' argument is nil
- 	;; (which seems to only be used by dired when updating its display
- 	;; after operating on a set of files).  We should change
- 	;; ange-ftp-insert-directory so that this case is handled by getting
- 	;; a full listing of the directory and extracting the line
- 	;; corresponding to the requested file.
  	(unless (equal cmd1 ".")
  	  (setq result (ange-ftp-cd host user (nth 1 cmd) 'noerror)))
  	(setq cmd1 cmd3)))
--- 2332,2337 ----
***************
*** 4498,4504 ****
  	   (if wildcard
  	       (let ((default-directory (file-name-directory file)))
  		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
! 	     (ange-ftp-ls file switches full))))
        (ange-ftp-real-insert-directory file switches wildcard full))))
  
  (defun ange-ftp-dired-uncache (dir)
--- 4488,4510 ----
  	   (if wildcard
  	       (let ((default-directory (file-name-directory file)))
  		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
! 	     ;; That `full' is nil means we're going to do `ls' for
! 	     ;; a single file in the current directory.
! 	     (if full
! 		 (ange-ftp-ls file switches full)
! 	       ;; Get a full listing of the current directory and
! 	       ;; extract the line corresponding to `file'.
! 	       (when (string-match "d\\'" switches)
! 		 ;; Remove "d" which dired added to `switches'.
! 		 (setq switches (substring switches 0 (match-beginning 0))))
! 	       (setq tem (ange-ftp-ls "." switches nil))
! 	       (let ((case-fold-search nil))
! 		 ;; Assume the file name which a single space precedes
! 		 ;; appears at the end of a line.
! 		 (if (string-match (concat "^.+[^ ] " (regexp-quote file) "$")
! 				   tem)
! 		     (concat (match-string 0 tem) "\n")
! 		   ""))))))
        (ange-ftp-real-insert-directory file switches wildcard full))))
  
  (defun ange-ftp-dired-uncache (dir)
--8<---------------cut here---------------end--------------->8---

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

>>>>> In <b4mk6hsdxe7.fsf@jpl.org> Katsumi Yamaoka wrote:

> *** ange-ftp.el~	Thu Aug 11 21:55:21 2005
> --- ange-ftp.el	Thu Sep  8 03:57:18 2005
> ***************
> *** 1656,1662 ****

>   	      ;; handle hash mark printing
>   	      (and ange-ftp-process-busy
> ! 		   (string-match "\\`#+\\'" str)
>   		   (setq str (ange-ftp-process-handle-hash str)))
>   	      (comint-output-filter proc str)
>   	      ;; Replace STR by the result of the comint processing.
> --- 1656,1662 ----

>   	      ;; handle hash mark printing
>   	      (and ange-ftp-process-busy
> ! 		   (string-match "#+" str)
>   		   (setq str (ange-ftp-process-handle-hash str)))
>   	      (comint-output-filter proc str)
>   	      ;; Replace STR by the result of the comint processing.

Please replace "#+" with the same "^#+$" as what is used in
Emacs 21.4.  While "\\`#+\\'" is intended to match the whole
string, `str' often contains newline characters.  This is the
true cause in which this function breaks the synchronous
operation.  I guess someone changed it mistakenly.

Re: tiny patch to ange-ftp

[Stefan Monnier]

> Please replace "#+" with the same "^#+$" as what is used in
> Emacs 21.4.  While "\\`#+\\'" is intended to match the whole
> string, `str' often contains newline characters.  This is the
> true cause in which this function breaks the synchronous
> operation.  I guess someone changed it mistakenly.

Yes, it was an error on my part, and I reverted it a few days ago,


        Stefan

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

Hi,

Could you apply the patch to ange-ftp.el?  I hope it is widely
tested by people who update-build Emacs everyday.  It enables at
least me to upload files to remote hosts smoothly and I believe
it doesn't worsen things than now.

2005-09-21  Katsumi Yamaoka  <yamaoka@jpl.org>  (tiny change)

	* net/ange-ftp.el (ange-ftp-insert-directory): Work with a single
	file listing.

*** ange-ftp.el~	Wed Sep 14 21:52:07 2005
--- ange-ftp.el	Wed Sep 21 23:39:58 2005
***************
*** 2324,2339 ****
  	;; This works around a misfeature of some versions of netbsd ftpd
  	;; where `ls' can only take one argument: either one set of flags
  	;; or a file/directory name.
- 	;; FIXME: if we're trying to `ls' a single file, this fails since we
- 	;; can't cd to a file.  We can't fix this problem here, tho, because
- 	;; at this point we don't know whether the argument is a file or
- 	;; a directory.  Such an `ls' is only every used (apparently) from
- 	;; `insert-directory' when the `full-directory-p' argument is nil
- 	;; (which seems to only be used by dired when updating its display
- 	;; after operating on a set of files).  We should change
- 	;; ange-ftp-insert-directory so that this case is handled by getting
- 	;; a full listing of the directory and extracting the line
- 	;; corresponding to the requested file.
  	(unless (equal cmd1 ".")
  	  (setq result (ange-ftp-cd host user (nth 1 cmd) 'noerror)))
  	(setq cmd1 cmd3)))
--- 2324,2329 ----
***************
*** 4489,4495 ****
  	   (if wildcard
  	       (let ((default-directory (file-name-directory file)))
  		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
! 	     (ange-ftp-ls file switches full))))
        (ange-ftp-real-insert-directory file switches wildcard full))))
  
  (defun ange-ftp-dired-uncache (dir)
--- 4479,4501 ----
  	   (if wildcard
  	       (let ((default-directory (file-name-directory file)))
  		 (ange-ftp-ls (file-name-nondirectory file) switches nil nil t))
! 	     ;; That `full' is nil means we're going to do `ls' for
! 	     ;; a single file in the current directory.
! 	     (if full
! 		 (ange-ftp-ls file switches full)
! 	       ;; Get a full listing of the current directory and
! 	       ;; extract the line corresponding to `file'.
! 	       (when (string-match "d\\'" switches)
! 		 ;; Remove "d" which dired added to `switches'.
! 		 (setq switches (substring switches 0 (match-beginning 0))))
! 	       (setq tem (ange-ftp-ls "." switches nil))
! 	       (let ((case-fold-search nil))
! 		 ;; Assume the file name which a single space precedes
! 		 ;; appears at the end of a line.
! 		 (if (string-match (concat "^.+[^ ] " (regexp-quote file) "$")
! 				   tem)
! 		     (concat (match-string 0 tem) "\n")
! 		   ""))))))
        (ange-ftp-real-insert-directory file switches wildcard full))))
  
  (defun ange-ftp-dired-uncache (dir)

Re: tiny patch to ange-ftp

[Stefan Monnier]

> Could you apply the patch to ange-ftp.el?  I hope it is widely
> tested by people who update-build Emacs everyday.  It enables at
> least me to upload files to remote hosts smoothly and I believe
> it doesn't worsen things than now.

I'll install a variant of it, Real-Soon-Now.
Sorry for taking so much time,


        Stefan

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

>>>>> In <878xxpxhvc.fsf-monnier+emacs@gnu.org> Stefan Monnier wrote:

>> Could you apply the patch to ange-ftp.el?  I hope it is widely
>> tested by people who update-build Emacs everyday.  It enables at
>> least me to upload files to remote hosts smoothly and I believe
>> it doesn't worsen things than now.

> I'll install a variant of it, Real-Soon-Now.
> Sorry for taking so much time,

I'm sorry to bother you, but the file in question seems to have
not been changed yet.  How about that?

Re: tiny patch to ange-ftp

[Stefan Monnier]

>>> Could you apply the patch to ange-ftp.el?  I hope it is widely
>>> tested by people who update-build Emacs everyday.  It enables at
>>> least me to upload files to remote hosts smoothly and I believe
>>> it doesn't worsen things than now.

>> I'll install a variant of it, Real-Soon-Now.
>> Sorry for taking so much time,

> I'm sorry to bother you, but the file in question seems to have
> not been changed yet.  How about that?

I've installed my change.  Sorry again for the delay.


        Stefan

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

>>>>> In <87k6gyi7vo.fsf-monnier+emacs@gnu.org>
>>>>>	Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I've installed my change.  Sorry again for the delay.

I rebuilt Emacs.  However, I still could not upload files with
another problem.  `ange-ftp-insert-directory' passes a file name
without directory components to `ange-ftp-get-file-entry' then.
So, the following change seems to be required (I confirmed it
did the trick):

*** ange-ftp.el~	Sat Oct  1 10:46:12 2005
--- ange-ftp.el	Sat Oct  1 12:21:20 2005
***************
*** 2972,2982 ****
  	 (file (ange-ftp-get-file-part name)))
      (if ent
  	(gethash file ent)
!       (or (and (ange-ftp-allow-child-lookup dir file)
  	       (setq ent (ange-ftp-get-files name t))
  	       (gethash "." ent))
  	  ;; i.e. it's a directory by child lookup
! 	  (and (setq ent (ange-ftp-get-files dir t))
  	       (gethash file ent))))))
  
  (defun ange-ftp-internal-delete-file-entry (name &optional dir-p)
--- 2972,2984 ----
  	 (file (ange-ftp-get-file-part name)))
      (if ent
  	(gethash file ent)
!       (or (and dir
! 	       (ange-ftp-allow-child-lookup dir file)
  	       (setq ent (ange-ftp-get-files name t))
  	       (gethash "." ent))
  	  ;; i.e. it's a directory by child lookup
! 	  (and dir
! 	       (setq ent (ange-ftp-get-files dir t))
  	       (gethash file ent))))))
  
  (defun ange-ftp-internal-delete-file-entry (name &optional dir-p)

Re: tiny patch to ange-ftp

[Stefan Monnier]

> I rebuilt Emacs.  However, I still could not upload files with
> another problem.  `ange-ftp-insert-directory' passes a file name
> without directory components to `ange-ftp-get-file-entry' then.

That's an error in ange-ftp-insert-directory.

> So, the following change seems to be required (I confirmed it
> did the trick):

It works around the bug, tho.  I'd rather fix it instead.
I've just installed a patch which should fix it.  Does it work for you?

Although the patch works (it fixes the symlink-following code), in reality,
I see no reason why we should follow symlinks at all.  The normal code for
insert-directory doesn't follow symlinks, so why should the ange-ftp
version of it do it?  It seems to be asking for trouble, especially in the
present case: what if dired wants to refresh a single line that contains
a symlink?

The change to follow symlinks has sadly no explanation:

   revision 1.20
   date: 2001/12/29 02:50:34;  author: rms;  state: Exp;  lines: +12 -6
   (ange-ftp-insert-directory): Explicitly follow symlinks.

In the ChangeLog file, we get a tiny bit more info:

	* net/ange-ftp.el (ange-ftp-allow-child-lookup): Always return nil.
	This fixes a bug that treated all files as directories.
	(ange-ftp-insert-directory): Explicitly follow symlinks.

But it's still not clear why we'd want to follow symlinks.  My suspicion is
that at that time, it was necessary to follow symlinks in order for the
ange-ftp-ls command to give us a listing of a directory rather than just the
single-line listing of a symlink.  In that case, this is not a problem any
more since ange-ftp-ls now does "cd foo; ls" rather than "ls foo".  So I'd
be happy to install the second patch.

To see one of the problems with the idea of following symlinks, try
something like

  (insert-directory "/ftp@ftp.gnu.org:/pub" "-lFd")

Of course, there's also the problem where the symlink points to itself.
And also the problem that

  (insert-directory "/ftp@ftp.gnu.org:/pub/gnu/winboard" "-lFd")

will not list

  lrwxrwxrwx    1 0        0              15 Nov 21  2003 winboard -> xboard/winboard

but

  drwxrwxr-x    2 0        1003         4096 Nov 29  2003 winboard/

Contrary to what it would have done if we were working locally.


        Stefan


--- ange-ftp.el	02 oct 2005 11:37:24 -0400	1.74
+++ ange-ftp.el	02 oct 2005 11:38:44 -0400	
@@ -4481,14 +4481,6 @@
 (defun ange-ftp-insert-directory (file switches &optional wildcard full)
   (if (not (ange-ftp-ftp-name (expand-file-name file)))
       (ange-ftp-real-insert-directory file switches wildcard full)
-    ;; Follow symlinks.
-    (let (tem)
-      (while (and (not wildcard)
-                  (stringp (setq tem (file-symlink-p
-                                      (directory-file-name file)))))
-        (setq file
-              (ange-ftp-expand-symlink
-               tem (file-name-directory (directory-file-name file))))))
     (insert
      (cond
       (wildcard

Re: tiny patch to ange-ftp

[Katsumi Yamaoka]

>>>>> In <87ll1chs0h.fsf-monnier+emacs@gnu.org>
>>>>>	Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I've just installed a patch which should fix it.  Does it work for you?

Thanks!  It works fine, but I prefer the second patch...

> Although the patch works (it fixes the symlink-following code), in reality,
> I see no reason why we should follow symlinks at all.  The normal code for
> insert-directory doesn't follow symlinks, so why should the ange-ftp
> version of it do it?  It seems to be asking for trouble, especially in the
> present case: what if dired wants to refresh a single line that contains
> a symlink?

> The change to follow symlinks has sadly no explanation:

>    revision 1.20
>    date: 2001/12/29 02:50:34;  author: rms;  state: Exp;  lines: +12 -6
>    (ange-ftp-insert-directory): Explicitly follow symlinks.

> In the ChangeLog file, we get a tiny bit more info:

> 	* net/ange-ftp.el (ange-ftp-allow-child-lookup): Always return nil.
> 	This fixes a bug that treated all files as directories.
> 	(ange-ftp-insert-directory): Explicitly follow symlinks.

> But it's still not clear why we'd want to follow symlinks.  My suspicion is
> that at that time, it was necessary to follow symlinks in order for the
> ange-ftp-ls command to give us a listing of a directory rather than just the
> single-line listing of a symlink.  In that case, this is not a problem any
> more since ange-ftp-ls now does "cd foo; ls" rather than "ls foo".  So I'd
> be happy to install the second patch.

I tried the second patch.  I think it is much better, and I can
agree that to follow symlinks there is meaningless.

> To see one of the problems with the idea of following symlinks, try
> something like

>   (insert-directory "/ftp@ftp.gnu.org:/pub" "-lFd")

> Of course, there's also the problem where the symlink points to itself.
> And also the problem that

>   (insert-directory "/ftp@ftp.gnu.org:/pub/gnu/winboard" "-lFd")

> will not list

>   lrwxrwxrwx    1 0        0              15 Nov 21  2003 winboard -> xboard/winboard

> but

>   drwxrwxr-x    2 0        1003         4096 Nov 29  2003 winboard/

> Contrary to what it would have done if we were working locally.

>         Stefan

I think concealing of file's identity is needless and unhelpful.

> --- ange-ftp.el	02 oct 2005 11:37:24 -0400	1.74
> +++ ange-ftp.el	02 oct 2005 11:38:44 -0400	
> @@ -4481,14 +4481,6 @@
>  (defun ange-ftp-insert-directory (file switches &optional wildcard full)
>    (if (not (ange-ftp-ftp-name (expand-file-name file)))
>        (ange-ftp-real-insert-directory file switches wildcard full)
> -    ;; Follow symlinks.
> -    (let (tem)
> -      (while (and (not wildcard)
> -                  (stringp (setq tem (file-symlink-p
> -                                      (directory-file-name file)))))
> -        (setq file
> -              (ange-ftp-expand-symlink
> -               tem (file-name-directory (directory-file-name file))))))
>      (insert
>       (cond
>        (wildcard