unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* ange-ftp-file-size
@ 2009-10-15  3:20 Toru TSUNEYOSHI
  2009-10-15 12:55 ` ange-ftp-file-size Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Toru TSUNEYOSHI @ 2009-10-15  3:20 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: Text/Plain, Size: 262 bytes --]

Hello.

Now, ange-ftp-file-attributes doesn't return the file size. It returns
-1.
I wish it returns the size exactly, so I make ange-ftp-file-size.
If no problem, would you like to apply to ange-ftp.el, and modify code
at the size of ange-ftp-file-attributes ?

[-- Attachment #2: ange-ftp-file-size.el --]
[-- Type: Text/Plain, Size: 1205 bytes --]

;; After the style of `ange-ftp-file-modtime'.
;; *** I don't check return code 226 on wu-ftpd. ***

(defun ange-ftp-file-size (file &optional ascii-mode)
  "Return the size of remote file FILE. Return -1 if can't get it.
If ascii-mode is non-nil, return the size with the extra octets that
need to be inserted, one at the end of each line, to provide correct
end-of-line semantics for a transfer using TYPE=A. The default is nil,
so return the size on the remote host exactly. See RFC 3659."
  (let* ((parsed (ange-ftp-ftp-name file))
	 (host (nth 0 parsed))
	 (user (nth 1 parsed))
	 (name (ange-ftp-quote-string (nth 2 parsed)))
	 ;; At least one FTP server (wu-ftpd) can return a "226
	 ;; Transfer complete" before the "213 SIZE".  Let's skip
	 ;; that.
	 (ange-ftp-skip-msgs (concat ange-ftp-skip-msgs "\\|^226"))
         (res (prog2
		  (unless ascii-mode
		    (ange-ftp-set-binary-mode host user))
		  (ange-ftp-send-cmd host user (list 'quote "size" name))
		(unless ascii-mode
		  (ange-ftp-set-ascii-mode host user))))
	 (line (cdr res))
	 (size -1))
    (save-match-data
      (when (string-match "^213 \\([0-9]+\\)$" line)
	(setq size (string-to-number (match-string 1 line)))))
    size))

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-15  3:20 ange-ftp-file-size Toru TSUNEYOSHI
@ 2009-10-15 12:55 ` Stefan Monnier
  2009-10-16  4:31   ` ange-ftp-file-size Toru TSUNEYOSHI
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2009-10-15 12:55 UTC (permalink / raw)
  To: Toru TSUNEYOSHI; +Cc: emacs-devel

> Now, ange-ftp-file-attributes doesn't return the file size. It returns
> -1.
> I wish it returns the size exactly, so I make ange-ftp-file-size.
> If no problem, would you like to apply to ange-ftp.el, and modify code
> at the size of ange-ftp-file-attributes ?

That sounds like a useful improvement, yes.  Could you send a patch to
do that?

[...]
> 	 (line (cdr res))
> 	 (size -1))
>     (save-match-data
>       (when (string-match "^213 \\([0-9]+\\)$" line)
> 	(setq size (string-to-number (match-string 1 line)))))
>     size))

And here comes the version, in functional programming style:

	 (line (cdr res)))
    (save-match-data
      (if (string-match "^213 \\([0-9]+\\)$" line)
          (string-to-number (match-string 1 line))
        -1))))

BTW: the save-match-data seems completely unnecessary.  Please remove
it, or add a clear comment explaining why it's needed.


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-15 12:55 ` ange-ftp-file-size Stefan Monnier
@ 2009-10-16  4:31   ` Toru TSUNEYOSHI
  2009-10-16 15:22     ` ange-ftp-file-size Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Toru TSUNEYOSHI @ 2009-10-16  4:31 UTC (permalink / raw)
  To: monnier; +Cc: emacs-devel

[-- Attachment #1: Type: Text/Plain, Size: 263 bytes --]

Thank you for your replying.

I revised the code on your advice, and made a patch of ange-ftp.el on
Emacs 23.1.1. Would you like to check it?

BTW, about save-match-data, I used it because of inhibiting changing old
match data. Using it is wrong (as the style) ?

[-- Attachment #2: ange-ftp.el.diff --]
[-- Type: Text/X-Patch, Size: 2141 bytes --]

--- ange-ftp.el.orig	Sun Jun 21 13:37:58 2009
+++ ange-ftp.el	Fri Oct 16 11:45:55 2009
@@ -2338,7 +2338,7 @@
 
      ;; Second argument is the remote name
      ((or (memq cmd0 '(append put chmod))
-          (and (eq cmd0 'quote) (string= cmd1 "mdtm")))
+          (and (eq cmd0 'quote) (string-match-p "^\\(mdtm\\|size\\)$" cmd1)))
       (setq cmd2 (funcall fix-name-func cmd2)))
      ;; Both arguments are remote names
      ((eq cmd0 'rename)
@@ -3455,7 +3455,7 @@
 		      '(0 0)		;4 atime
 		      (ange-ftp-file-modtime file) ;5 mtime
 		      '(0 0)		;6 ctime
-		      -1		;7 size
+		      (ange-ftp-file-size file)	;7 size
 		      (concat (if (stringp dirp) "l" (if dirp "d" "-"))
 			      "?????????") ;8 mode
 		      nil		;9 gid weird
@@ -3557,6 +3557,32 @@
           (or (zerop (car file-mdtm))
               (<= (float-time file-mdtm) (float-time buf-mdtm))))
       (ange-ftp-real-verify-visited-file-modtime buf))))
+
+(defun ange-ftp-file-size (file &optional ascii-mode)
+  "Return the size of remote file FILE. Return -1 if can't get it.
+If ascii-mode is non-nil, return the size with the extra octets that
+need to be inserted, one at the end of each line, to provide correct
+end-of-line semantics for a transfer using TYPE=A. The default is nil,
+so return the size on the remote host exactly. See RFC 3659."
+  (let* ((parsed (ange-ftp-ftp-name file))
+	 (host (nth 0 parsed))
+	 (user (nth 1 parsed))
+	 (name (ange-ftp-quote-string (nth 2 parsed)))
+	 ;; At least one FTP server (wu-ftpd) can return a "226
+	 ;; Transfer complete" before the "213 SIZE".  Let's skip
+	 ;; that.
+	 (ange-ftp-skip-msgs (concat ange-ftp-skip-msgs "\\|^226"))
+	 (res (prog2
+		  (unless ascii-mode
+		    (ange-ftp-set-binary-mode host user))
+		  (ange-ftp-send-cmd host user (list 'quote "size" name))
+		(unless ascii-mode
+		  (ange-ftp-set-ascii-mode host user))))
+	 (line (cdr res)))
+    (if (string-match "^213 \\([0-9]+\\)$" line)
+	(string-to-number (match-string 1 line))
+      -1)))
+
 \f
 ;;;; ------------------------------------------------------------
 ;;;; File copying support... totally re-written 6/24/92.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-16  4:31   ` ange-ftp-file-size Toru TSUNEYOSHI
@ 2009-10-16 15:22     ` Stefan Monnier
  2009-10-17  2:21       ` ange-ftp-file-size Toru TSUNEYOSHI
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2009-10-16 15:22 UTC (permalink / raw)
  To: Toru TSUNEYOSHI; +Cc: emacs-devel

> BTW, about save-match-data, I used it because of inhibiting changing old
> match data. Using it is wrong (as the style)?

save-match-data should only be used where it is really needed, not
defensively: 99% of the code may destroy the match-date (and could thus
use save-match-data defensively), whereas less than 1% of the code needs
the match-data to be saved.

> -          (and (eq cmd0 'quote) (string= cmd1 "mdtm")))
> +          (and (eq cmd0 'quote) (string-match-p "^\\(mdtm\\|size\\)$" cmd1)))

I used (member cmd1 '("mdtm" "size")) instead.  The regexp equivalent
would be "\\`\\(mdtm\\|size\\)\\'".

> +	 (res (prog2
> +		  (unless ascii-mode
> +		    (ange-ftp-set-binary-mode host user))
> +		  (ange-ftp-send-cmd host user (list 'quote "size" name))
> +		(unless ascii-mode
> +		  (ange-ftp-set-ascii-mode host user))))

Now that I look at it: isn't it problematic to set the mode to `ascii'
here?  If the mode was binary before and we call it with ascii-mode=nil,
we end up switching to ascii mode unwittingly, don't we?


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-16 15:22     ` ange-ftp-file-size Stefan Monnier
@ 2009-10-17  2:21       ` Toru TSUNEYOSHI
  2009-10-18  2:03         ` ange-ftp-file-size Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Toru TSUNEYOSHI @ 2009-10-17  2:21 UTC (permalink / raw)
  To: monnier; +Cc: emacs-devel

[-- Attachment #1: Type: Text/Plain, Size: 2030 bytes --]

Thank you for your checking.

>> BTW, about save-match-data, I used it because of inhibiting changing old
>> match data. Using it is wrong (as the style)?
> 
> save-match-data should only be used where it is really needed, not
> defensively: 99% of the code may destroy the match-date (and could thus
> use save-match-data defensively), whereas less than 1% of the code needs
> the match-data to be saved.
> 

I understand you. I will be carefull about using `save-match-data' from
now on.

>> -          (and (eq cmd0 'quote) (string= cmd1 "mdtm")))
>> +          (and (eq cmd0 'quote) (string-match-p "^\\(mdtm\\|size\\)$" cmd1)))
> 
> I used (member cmd1 '("mdtm" "size")) instead.  The regexp equivalent
> would be "\\`\\(mdtm\\|size\\)\\'".
> 

I also think it is better.

>> +	 (res (prog2
>> +		  (unless ascii-mode
>> +		    (ange-ftp-set-binary-mode host user))
>> +		  (ange-ftp-send-cmd host user (list 'quote "size" name))
>> +		(unless ascii-mode
>> +		  (ange-ftp-set-ascii-mode host user))))
> 
> Now that I look at it: isn't it problematic to set the mode to `ascii'
> here?  If the mode was binary before and we call it with ascii-mode=nil,
> we end up switching to ascii mode unwittingly, don't we?
> 

I read the code of `ange-ftp.el'.
The follows is written...

    In comments:

        By default ange-ftp transfers files in ASCII mode.

    In code:

        (defun ange-ftp-...
            ...
              (unwind-protect
                  (progn
                    ...
                    (if binary
                        (ange-ftp-set-binary-mode host user))
                    ...)
                (if binary
                    (ange-ftp-set-ascii-mode host user)))
            ...)

So, I understood that `ASCII mode' is basic mode.
I wrote after the style.
(In my former patch, I should have used `unwind-protect'.)

But, as you wrote, there is no guarantee that the mode is always ascii.
So, I wrote the mode changing clearly.

> 
>         Stefan
> 

Would you like to check my patch again ?

[-- Attachment #2: ange-ftp.el.diff --]
[-- Type: Text/X-Patch, Size: 2206 bytes --]

--- ange-ftp.el.orig	Sun Jun 21 13:37:58 2009
+++ ange-ftp.el	Sat Oct 17 11:06:58 2009
@@ -2338,7 +2338,7 @@
 
      ;; Second argument is the remote name
      ((or (memq cmd0 '(append put chmod))
-          (and (eq cmd0 'quote) (string= cmd1 "mdtm")))
+          (and (eq cmd0 'quote) (member cmd1 '("mdtm" "size"))))
       (setq cmd2 (funcall fix-name-func cmd2)))
      ;; Both arguments are remote names
      ((eq cmd0 'rename)
@@ -3455,7 +3455,7 @@
 		      '(0 0)		;4 atime
 		      (ange-ftp-file-modtime file) ;5 mtime
 		      '(0 0)		;6 ctime
-		      -1		;7 size
+		      (ange-ftp-file-size file)	;7 size
 		      (concat (if (stringp dirp) "l" (if dirp "d" "-"))
 			      "?????????") ;8 mode
 		      nil		;9 gid weird
@@ -3557,6 +3557,35 @@
           (or (zerop (car file-mdtm))
               (<= (float-time file-mdtm) (float-time buf-mdtm))))
       (ange-ftp-real-verify-visited-file-modtime buf))))
+
+(defun ange-ftp-file-size (file &optional ascii-mode)
+  "Return the size of remote file FILE. Return -1 if can't get it.
+If ascii-mode is non-nil, return the size with the extra octets that
+need to be inserted, one at the end of each line, to provide correct
+end-of-line semantics for a transfer using TYPE=A. The default is nil,
+so return the size on the remote host exactly. See RFC 3659."
+  (let* ((parsed (ange-ftp-ftp-name file))
+	 (host (nth 0 parsed))
+	 (user (nth 1 parsed))
+	 (name (ange-ftp-quote-string (nth 2 parsed)))
+	 ;; At least one FTP server (wu-ftpd) can return a "226
+	 ;; Transfer complete" before the "213 SIZE".  Let's skip
+	 ;; that.
+	 (ange-ftp-skip-msgs (concat ange-ftp-skip-msgs "\\|^226"))
+	 res line)
+    (unwind-protect
+	(progn
+	  (if ascii-mode
+	      (ange-ftp-set-ascii-mode host user)
+	    (ange-ftp-set-binary-mode host user))
+	  (setq res (ange-ftp-send-cmd host user (list 'quote "size" name))
+		line (cdr res))
+	  (if (string-match "^213 \\([0-9]+\\)$" line)
+	      (string-to-number (match-string 1 line))
+	    -1))
+      (unless ascii-mode
+	(ange-ftp-set-ascii-mode host user)))))
+
 \f
 ;;;; ------------------------------------------------------------
 ;;;; File copying support... totally re-written 6/24/92.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-17  2:21       ` ange-ftp-file-size Toru TSUNEYOSHI
@ 2009-10-18  2:03         ` Stefan Monnier
  2009-10-18  3:07           ` ange-ftp-file-size Toru TSUNEYOSHI
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2009-10-18  2:03 UTC (permalink / raw)
  To: Toru TSUNEYOSHI; +Cc: emacs-devel

>> Now that I look at it: isn't it problematic to set the mode to `ascii'
>> here?  If the mode was binary before and we call it with ascii-mode=nil,
>> we end up switching to ascii mode unwittingly, don't we?

> I read the code of `ange-ftp.el'.
> The follows is written...

>     In comments:

>         By default ange-ftp transfers files in ASCII mode.

>     In code:

>         (defun ange-ftp-...
>             ...
>               (unwind-protect
>                   (progn
>                     ...
>                     (if binary
>                         (ange-ftp-set-binary-mode host user))
>                     ...)
>                 (if binary
>                     (ange-ftp-set-ascii-mode host user)))
>             ...)

> So, I understood that `ASCII mode' is basic mode.
> I wrote after the style.
> (In my former patch, I should have used `unwind-protect'.)

OK, thanks, I've changed the prog2 to an unwind-protect.


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ange-ftp-file-size
  2009-10-18  2:03         ` ange-ftp-file-size Stefan Monnier
@ 2009-10-18  3:07           ` Toru TSUNEYOSHI
  0 siblings, 0 replies; 7+ messages in thread
From: Toru TSUNEYOSHI @ 2009-10-18  3:07 UTC (permalink / raw)
  To: monnier; +Cc: emacs-devel

> OK, thanks, I've changed the prog2 to an unwind-protect.
> 
> 
>         Stefan
> 

Thanks for your support.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-10-18  3:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-15  3:20 ange-ftp-file-size Toru TSUNEYOSHI
2009-10-15 12:55 ` ange-ftp-file-size Stefan Monnier
2009-10-16  4:31   ` ange-ftp-file-size Toru TSUNEYOSHI
2009-10-16 15:22     ` ange-ftp-file-size Stefan Monnier
2009-10-17  2:21       ` ange-ftp-file-size Toru TSUNEYOSHI
2009-10-18  2:03         ` ange-ftp-file-size Stefan Monnier
2009-10-18  3:07           ` ange-ftp-file-size Toru TSUNEYOSHI

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).