* after-find-file-from-revert-buffer
@ 2011-04-06 0:28 Juanma Barranquero
2011-04-06 1:34 ` after-find-file-from-revert-buffer Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juanma Barranquero @ 2011-04-06 0:28 UTC (permalink / raw
To: Emacs developers
I'm using lexical-binding = t to detect cases of unused arguments and
variables, unwanted shadowing of global variables, etc. (I'm not
planning to convert more packages to lexical-binding, just doing part
of the footwork in case someone wants to do it later.)
In function `after-find-file', one of the parameters is
AFTER-FIND-FILE-FROM-REVERT-BUFFER, whose purpose is to bind that
(undocumented) variable so code run from inside `after-find-file' can
detect whether the function was called from revert-buffer or not.
Its only use, now and since the parameter's and variable's
introduction sixteen years ago, is in the `saveplace-find-file-hook'
function, run from `find-file-hook', to decide whether to move the
cursor or not.
So I want to rename the argument to CALLED-FROM-REVERT-BUFFER-P, and
use a let inside `after-find-file'
(let ((after-find-file-from-revert-buffer called-from-revert-buffer-p))
;; whatever
...)
To strictly keep the current semantics, the "whatever" inside that
`let' should be all the current code of `after-find-file'. But that
seems ugly and wasteful, and I think we can go forth with just
let-binding the inside of the final (unless nomodes ...) form:
(unless nomodes
(let ((after-find-file-from-revert-buffer called-from-revert-buffer-p))
(when (and view-read-only view-mode)
(view-mode-disable))
(normal-mode t)
;; If requested, add a newline at the end of the file.
(and (memq require-final-newline '(visit visit-save))
(> (point-max) (point-min))
(/= (char-after (1- (point-max))) ?\n)
(not (and (eq selective-display t)
(= (char-after (1- (point-max))) ?\r)))
(save-excursion
(goto-char (point-max))
(insert "\n")))
(when (and buffer-read-only
view-read-only
(not (eq (get major-mode 'mode-class) 'special)))
(view-mode-enter))
(run-hooks 'find-file-hook))))
Additionally, we could document the variable, or find another, less
ugly way to pass that information back to saveplace.el and remove the
variable altogether.
Comments?
TIA,
Juanma
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 0:28 after-find-file-from-revert-buffer Juanma Barranquero
@ 2011-04-06 1:34 ` Stefan Monnier
2011-04-06 1:42 ` after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 15:18 ` after-find-file-from-revert-buffer Juanma Barranquero
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Monnier @ 2011-04-06 1:34 UTC (permalink / raw
To: Juanma Barranquero; +Cc: Emacs developers
> Additionally, we could document the variable, or find another, less
> ugly way to pass that information back to saveplace.el and remove the
> variable altogether.
Removing the variable sounds like the best option. So the question is
how to get the same behavior in saveplace without using this variable.
Maybe saveplace could setup a local revert-buffer-function instead.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 1:34 ` after-find-file-from-revert-buffer Stefan Monnier
@ 2011-04-06 1:42 ` Juanma Barranquero
2011-04-06 15:17 ` after-find-file-from-revert-buffer Stefan Monnier
2011-04-06 15:18 ` after-find-file-from-revert-buffer Juanma Barranquero
1 sibling, 1 reply; 9+ messages in thread
From: Juanma Barranquero @ 2011-04-06 1:42 UTC (permalink / raw
To: Stefan Monnier; +Cc: Emacs developers
On Wed, Apr 6, 2011 at 03:34, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> Maybe saveplace could setup a local revert-buffer-function instead.
I'll look into that.
If that works, what to do with 4th arg of `after-find-file'? Keep it
(and ignore it), or remove it and use
`set-advertised-calling-convention'?
Juanma
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 1:42 ` after-find-file-from-revert-buffer Juanma Barranquero
@ 2011-04-06 15:17 ` Stefan Monnier
2011-04-06 17:43 ` after-find-file-from-revert-buffer Juanma Barranquero
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2011-04-06 15:17 UTC (permalink / raw
To: Juanma Barranquero; +Cc: Emacs developers
>> Maybe saveplace could setup a local revert-buffer-function instead.
> I'll look into that.
Thanks.
> If that works, what to do with 4th arg of `after-find-file'? Keep it
> (and ignore it), or remove it and use
> `set-advertised-calling-convention'?
Keep it, ignore it, and set-advertised-calling-convention.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 1:34 ` after-find-file-from-revert-buffer Stefan Monnier
2011-04-06 1:42 ` after-find-file-from-revert-buffer Juanma Barranquero
@ 2011-04-06 15:18 ` Juanma Barranquero
2011-04-06 16:51 ` after-find-file-from-revert-buffer Stefan Monnier
1 sibling, 1 reply; 9+ messages in thread
From: Juanma Barranquero @ 2011-04-06 15:18 UTC (permalink / raw
To: Stefan Monnier; +Cc: Emacs developers
> Removing the variable sounds like the best option. So the question is
> how to get the same behavior in saveplace without using this variable.
> Maybe saveplace could setup a local revert-buffer-function instead.
The issue is that saveplace does not need or want a
revert-buffer-function. What it wants is to piggyback on
find-file-hook to restore the point in other buffers, but detect the
cases where that's caused by revert-buffer, to avoid unexpectedly
moving the point.
If the (before|after)-revert-hooks were always run, we could try to
use these (though it would be ugly); but a `revert-buffer-function' is
free to ignore them.
On the other hand, if the `revert-buffer-function' does not call
`after-find-file', or if it does but does not pass the
AFTER-FIND-FILE-FROM-REVERT-BUFFER parameter, the saveplace hook isn't
detecting the revert-buffer call anyway...
The only answer I can think is having something like
after-find-file-from-revert-buffer, but explicitly for revert-buffer:
(defvar revert-buffer-in-progress-p nil
"This variable is non-nil whenever a `revert-buffer' operation is
in progress, nil otherwise.")
(defun revert-buffer (...)
(let ((revert-buffer-in-progress t))
...))
which isn't not much better that which we have today, but at least is
better focused: it's cleaner to have a flag to signal use of
revert-buffer, than one to signal use of after-find-file inside
revert-buffer...
Juanma
P.S. Well, it would be more like
@@ -5047,8 +5048,10 @@
(interactive (list (not current-prefix-arg)))
(if revert-buffer-function
- (funcall revert-buffer-function ignore-auto noconfirm)
+ (let ((revert-buffer-in-progress-p t))
+ (funcall revert-buffer-function ignore-auto noconfirm))
(with-current-buffer (or (buffer-base-buffer (current-buffer))
(current-buffer))
- (let* ((auto-save-p (and (not ignore-auto)
+ (let* ((revert-buffer-in-progress-p t)
+ (auto-save-p (and (not ignore-auto)
(recent-auto-save-p)
buffer-auto-save-file-name
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 15:18 ` after-find-file-from-revert-buffer Juanma Barranquero
@ 2011-04-06 16:51 ` Stefan Monnier
2011-04-06 17:03 ` after-find-file-from-revert-buffer Juanma Barranquero
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2011-04-06 16:51 UTC (permalink / raw
To: Juanma Barranquero; +Cc: Emacs developers
> The issue is that saveplace does not need or want a
> revert-buffer-function. What it wants is to piggyback on
> find-file-hook to restore the point in other buffers, but detect the
> cases where that's caused by revert-buffer, to avoid unexpectedly
> moving the point.
I was thinking of using a revert-buffer-function of the form
(λ (&rest args)
(let ((save-place-in-revert-buffer t))
(apply revert-buffer-default args)))
> (defun revert-buffer (...)
> (let ((revert-buffer-in-progress t))
> ...))
> which isn't not much better that which we have today, but at least is
> better focused: it's cleaner to have a flag to signal use of
> revert-buffer, than one to signal use of after-find-file inside
> revert-buffer...
Yes, that's is better.
> @@ -5047,8 +5048,10 @@
> (interactive (list (not current-prefix-arg)))
> (if revert-buffer-function
> - (funcall revert-buffer-function ignore-auto noconfirm)
> + (let ((revert-buffer-in-progress-p t))
> + (funcall revert-buffer-function ignore-auto noconfirm))
> (with-current-buffer (or (buffer-base-buffer (current-buffer))
> (current-buffer))
> - (let* ((auto-save-p (and (not ignore-auto)
> + (let* ((revert-buffer-in-progress-p t)
> + (auto-save-p (and (not ignore-auto)
> (recent-auto-save-p)
> buffer-auto-save-file-name
I guess you could use a single let that covers both branches of the `if'.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 16:51 ` after-find-file-from-revert-buffer Stefan Monnier
@ 2011-04-06 17:03 ` Juanma Barranquero
2011-04-06 17:32 ` after-find-file-from-revert-buffer Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juanma Barranquero @ 2011-04-06 17:03 UTC (permalink / raw
To: Stefan Monnier; +Cc: Emacs developers
On Wed, Apr 6, 2011 at 18:51, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> I was thinking of using a revert-buffer-function of the form
>
> (λ (&rest args)
> (let ((save-place-in-revert-buffer t))
> (apply revert-buffer-default args)))
>
>> (defun revert-buffer (...)
>> (let ((revert-buffer-in-progress t))
>> ...))
Global, you mean? I'm not sure it's worth the hassle.
> Yes, that's is better.
OK, I'll install that.
> I guess you could use a single let that covers both branches of the `if'.
IMO, it seems ugly to wrap the whole function just to add a variable.
But it's your call.
Juanma
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 17:03 ` after-find-file-from-revert-buffer Juanma Barranquero
@ 2011-04-06 17:32 ` Stefan Monnier
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Monnier @ 2011-04-06 17:32 UTC (permalink / raw
To: Juanma Barranquero; +Cc: Emacs developers
>> Yes, that's is better.
> OK, I'll install that.
Fine,
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: after-find-file-from-revert-buffer
2011-04-06 15:17 ` after-find-file-from-revert-buffer Stefan Monnier
@ 2011-04-06 17:43 ` Juanma Barranquero
0 siblings, 0 replies; 9+ messages in thread
From: Juanma Barranquero @ 2011-04-06 17:43 UTC (permalink / raw
To: Stefan Monnier; +Cc: Emacs developers
> Keep it, ignore it, and set-advertised-calling-convention
The argument that is ignored is not the last one, so
`set-advertised-calling-convention' doesn't seem to be useful in this
case, unless we play games with the parameters.
Juanma
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-04-06 17:43 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-06 0:28 after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 1:34 ` after-find-file-from-revert-buffer Stefan Monnier
2011-04-06 1:42 ` after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 15:17 ` after-find-file-from-revert-buffer Stefan Monnier
2011-04-06 17:43 ` after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 15:18 ` after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 16:51 ` after-find-file-from-revert-buffer Stefan Monnier
2011-04-06 17:03 ` after-find-file-from-revert-buffer Juanma Barranquero
2011-04-06 17:32 ` after-find-file-from-revert-buffer 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).