Large-file check in files.el

Large-file check in files.el

[Adrian Robert]

Hi,

In find-file-noselect, confirmation is required for filesize >
large-file-warning-threshold, yet in insert-file-1, it is not.  Is
there a reason large files should be allowed to be inserted -- but not
loaded by themselves -- without confirmation?  If not, I propose the
patch below.

thanks,
Adrian
------------

Index: files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.966
diff -c -b -r1.966 files.el
*** files.el	14 Mar 2008 17:14:09 -0000	1.966
--- files.el	1 Apr 2008 04:55:25 -0000
***************
*** 1796,1801 ****
--- 1796,1811 ----
    (if (file-directory-p filename)
        (signal 'file-error (list "Opening input file" "file is a directory"
                                  filename)))
+   ;; Check whether the file is uncommonly large (see find-file-noselect):
+   (let (size)
+     (when (and large-file-warning-threshold
+ 	       (setq size (nth 7 (file-attributes filename)))
+ 	       (> size large-file-warning-threshold)
+ 	       (not (y-or-n-p
+ 		     (format "File %s is large (%dMB), really insert? "
+ 			     (file-name-nondirectory filename)
+ 			     (/ size 1048576)))))
+       (error "Aborted")))
    (let* ((buffer (find-buffer-visiting (abbreviate-file-name
(file-truename filename))
                                         #'buffer-modified-p))
           (tem (funcall insert-func filename)))


Diffs between working revision and workfile end here.

Re: Large-file check in files.el

[Stefan Monnier]

> In find-file-noselect, confirmation is required for filesize >
> large-file-warning-threshold, yet in insert-file-1, it is not.  Is
> there a reason large files should be allowed to be inserted -- but not
> loaded by themselves -- without confirmation?  If not, I propose the
> patch below.

That makes sense.  But please factor out the size-check code into its
own function which you can then call from both places.  This will be
better than duplicating the code like your patch currently does.


        Stefan


> Index: files.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
> retrieving revision 1.966
> diff -c -b -r1.966 files.el
> *** files.el	14 Mar 2008 17:14:09 -0000	1.966
> --- files.el	1 Apr 2008 04:55:25 -0000
> ***************
> *** 1796,1801 ****
> --- 1796,1811 ----
>     (if (file-directory-p filename)
>         (signal 'file-error (list "Opening input file" "file is a directory"
>                                   filename)))
> +   ;; Check whether the file is uncommonly large (see find-file-noselect):
> +   (let (size)
> +     (when (and large-file-warning-threshold
> + 	       (setq size (nth 7 (file-attributes filename)))
> + 	       (> size large-file-warning-threshold)
> + 	       (not (y-or-n-p
> + 		     (format "File %s is large (%dMB), really insert? "
> + 			     (file-name-nondirectory filename)
> + 			     (/ size 1048576)))))
> +       (error "Aborted")))
>     (let* ((buffer (find-buffer-visiting (abbreviate-file-name
> (file-truename filename))
>                                          #'buffer-modified-p))
>            (tem (funcall insert-func filename)))


> Diffs between working revision and workfile end here.

Re: Large-file check in files.el

[Adrian Robert]

On Tue, Apr 1, 2008 at 8:57 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > In find-file-noselect, confirmation is required for filesize >
>  > large-file-warning-threshold, yet in insert-file-1, it is not.  Is
>  > there a reason large files should be allowed to be inserted -- but not
>  > loaded by themselves -- without confirmation?  If not, I propose the
>  > patch below.
>
>  That makes sense.  But please factor out the size-check code into its
>  own function which you can then call from both places.  This will be
>  better than duplicating the code like your patch currently does.

OK, here is a new version of the patch.
------------------


Index: files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.966
diff -c -b -r1.966 files.el
*** files.el	14 Mar 2008 17:14:09 -0000	1.966
--- files.el	1 Apr 2008 12:00:30 -0000
***************
*** 1507,1512 ****
--- 1507,1523 ----
    :version "22.1"
    :type '(choice integer (const :tag "Never request confirmation" nil)))

+ (defun should-abort-large-file (attributes opType &optional buf nowarn)
+   "Utility function: return t if file is larger than
large-file-warning-threshold
+ and user declines loading it.  If buf or nowarn are true, always return nil."
+   (if (and large-file-warning-threshold (nth 7 attributes) (not (or
buf nowarn)))
+       (and (> (nth 7 attributes) large-file-warning-threshold)
+ 	   (not (y-or-n-p
+ 		 (format "File %s is large (%dMB), really %s? "
+ 			 (file-name-nondirectory filename)
+ 			 (/ (nth 7 attributes) 1048576) opType))))
+     nil))
+
  (defun find-file-noselect (filename &optional nowarn rawfile wildcards)
    "Read file FILENAME into a buffer and return the buffer.
  If a buffer exists visiting FILENAME, return that one, but
***************
*** 1558,1572 ****
  	      (if (or find-file-existing-other-name find-file-visit-truename)
  		  (setq buf other))))
  	;; Check to see if the file looks uncommonly large.
! 	(when (and large-file-warning-threshold (nth 7 attributes)
! 		   ;; Don't ask again if we already have the file or
! 		   ;; if we're asked to be quiet.
! 		   (not (or buf nowarn))
! 		   (> (nth 7 attributes) large-file-warning-threshold)
! 		   (not (y-or-n-p
! 			 (format "File %s is large (%dMB), really open? "
! 				 (file-name-nondirectory filename)
! 				   (/ (nth 7 attributes) 1048576)))))
  	  (error "Aborted"))
  	(if buf
  	    ;; We are using an existing buffer.
--- 1569,1575 ----
  	      (if (or find-file-existing-other-name find-file-visit-truename)
  		  (setq buf other))))
  	;; Check to see if the file looks uncommonly large.
! 	(when (should-abort-large-file attributes "open" buf nowarn)
  	  (error "Aborted"))
  	(if buf
  	    ;; We are using an existing buffer.
***************
*** 1796,1801 ****
--- 1799,1807 ----
    (if (file-directory-p filename)
        (signal 'file-error (list "Opening input file" "file is a directory"
                                  filename)))
+   ;; Check whether the file is uncommonly large (see find-file-noselect):
+   (when (should-abort-large-file (file-attributes filename) "insert")
+     (error "Aborted"))
    (let* ((buffer (find-buffer-visiting (abbreviate-file-name
(file-truename filename))
                                         #'buffer-modified-p))
           (tem (funcall insert-func filename)))


Diffs between working revision and workfile end here.

Re: Large-file check in files.el

[Stefan Monnier]

> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
> retrieving revision 1.966
> diff -c -b -r1.966 files.el
> *** files.el	14 Mar 2008 17:14:09 -0000	1.966
> --- files.el	1 Apr 2008 12:00:30 -0000
> ***************
> *** 1507,1512 ****
> --- 1507,1523 ----
>     :version "22.1"
>     :type '(choice integer (const :tag "Never request confirmation" nil)))

> + (defun should-abort-large-file (attributes opType &optional buf nowarn)
> +   "Utility function: return t if file is larger than
> large-file-warning-threshold
> + and user declines loading it.  If buf or nowarn are true, always return nil."
> +   (if (and large-file-warning-threshold (nth 7 attributes) (not (or
> buf nowarn)))
> +       (and (> (nth 7 attributes) large-file-warning-threshold)
> + 	   (not (y-or-n-p
> + 		 (format "File %s is large (%dMB), really %s? "
> + 			 (file-name-nondirectory filename)
> + 			 (/ (nth 7 attributes) 1048576) opType))))
> +     nil))

I'd call it "warn-about-large-file" or something like that and would let
it do the (error "Aborted!").

Please try C-u M-x checkdoc-current-buffer RET
Don't take its recommendations are gospel (even it tends to want to
force it upon you), but they're often useful for people to learn about
conventions, e.g. the first line of a docstring should be
self-contained, and arguments should appear in upper-case.
Also in Elisp, we tend to use "op-type" rather than "opType".
Finally "buf" shouldn't be an argument (only "nowarn" is needed).


        Stefan

Re: Large-file check in files.el

[Adrian Robert]

>  I'd call it "warn-about-large-file" or something like that and would let
>  it do the (error "Aborted!").

I did this and improved the doc.

------------

Index: files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.966
diff -c -b -r1.966 files.el
*** files.el	14 Mar 2008 17:14:09 -0000	1.966
--- files.el	2 Apr 2008 16:57:05 -0000
***************
*** 1507,1512 ****
--- 1507,1523 ----
    :version "22.1"
    :type '(choice integer (const :tag "Never request confirmation" nil)))

+ (defun abort-if-file-too-large (size op-type)
+   "If file SIZE larger than LARGE-FILE-WARNING-THRESHOLD, allow user to abort.
+ OP-TYPE specifies the file operation being performed (for message to user)."
+   (when (and large-file-warning-threshold size
+ 	   (> size large-file-warning-threshold)
+ 	   (not (y-or-n-p
+ 		 (format "File %s is large (%dMB), really %s? "
+ 			 (file-name-nondirectory filename)
+ 			 (/ size 1048576) op-type))))
+ 	  (error "Aborted")))
+
  (defun find-file-noselect (filename &optional nowarn rawfile wildcards)
    "Read file FILENAME into a buffer and return the buffer.
  If a buffer exists visiting FILENAME, return that one, but
***************
*** 1558,1573 ****
  	      (if (or find-file-existing-other-name find-file-visit-truename)
  		  (setq buf other))))
  	;; Check to see if the file looks uncommonly large.
! 	(when (and large-file-warning-threshold (nth 7 attributes)
! 		   ;; Don't ask again if we already have the file or
! 		   ;; if we're asked to be quiet.
! 		   (not (or buf nowarn))
! 		   (> (nth 7 attributes) large-file-warning-threshold)
! 		   (not (y-or-n-p
! 			 (format "File %s is large (%dMB), really open? "
! 				 (file-name-nondirectory filename)
! 				   (/ (nth 7 attributes) 1048576)))))
! 	  (error "Aborted"))
  	(if buf
  	    ;; We are using an existing buffer.
  	    (let (nonexistent)
--- 1569,1576 ----
  	      (if (or find-file-existing-other-name find-file-visit-truename)
  		  (setq buf other))))
  	;; Check to see if the file looks uncommonly large.
! 	(when (not (or buf nowarn))
! 	  (abort-if-file-too-large (nth 7 attributes) "open"))
  	(if buf
  	    ;; We are using an existing buffer.
  	    (let (nonexistent)
***************
*** 1796,1801 ****
--- 1799,1806 ----
    (if (file-directory-p filename)
        (signal 'file-error (list "Opening input file" "file is a directory"
                                  filename)))
+   ;; Check whether the file is uncommonly large
+   (abort-if-file-too-large (nth 7 (file-attributes filename)) "insert")
    (let* ((buffer (find-buffer-visiting (abbreviate-file-name
(file-truename filename))
                                         #'buffer-modified-p))
           (tem (funcall insert-func filename)))

Re: Large-file check in files.el

[Stefan Monnier]

>> I'd call it "warn-about-large-file" or something like that and would let
>> it do the (error "Aborted!").

> I did this and improved the doc.

Looks good, please install it with a proper changelog.


        Stefan