* Large-file check in files.el
@ 2008-04-01 4:55 Adrian Robert
2008-04-01 5:57 ` Stefan Monnier
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Robert @ 2008-04-01 4:55 UTC (permalink / raw)
To: emacs- devel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Large-file check in files.el
2008-04-01 4:55 Large-file check in files.el Adrian Robert
@ 2008-04-01 5:57 ` Stefan Monnier
2008-04-01 12:02 ` Adrian Robert
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2008-04-01 5:57 UTC (permalink / raw)
To: Adrian Robert; +Cc: emacs- devel
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Large-file check in files.el
2008-04-01 5:57 ` Stefan Monnier
@ 2008-04-01 12:02 ` Adrian Robert
2008-04-01 15:56 ` Stefan Monnier
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Robert @ 2008-04-01 12:02 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs- devel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Large-file check in files.el
2008-04-01 12:02 ` Adrian Robert
@ 2008-04-01 15:56 ` Stefan Monnier
2008-04-02 17:59 ` Adrian Robert
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2008-04-01 15:56 UTC (permalink / raw)
To: Adrian Robert; +Cc: emacs- devel
> ===================================================================
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Large-file check in files.el
2008-04-01 15:56 ` Stefan Monnier
@ 2008-04-02 17:59 ` Adrian Robert
2008-04-03 14:03 ` Stefan Monnier
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Robert @ 2008-04-02 17:59 UTC (permalink / raw)
To: emacs- devel
> 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)))
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-04-03 14:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-01 4:55 Large-file check in files.el Adrian Robert
2008-04-01 5:57 ` Stefan Monnier
2008-04-01 12:02 ` Adrian Robert
2008-04-01 15:56 ` Stefan Monnier
2008-04-02 17:59 ` Adrian Robert
2008-04-03 14:03 ` Stefan Monnier
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).