* save-excursion and save-current-buffer - edits to the manual @ 2011-03-13 16:24 Uday S Reddy 2011-03-13 18:27 ` Drew Adams 2011-03-19 20:36 ` Chong Yidong 0 siblings, 2 replies; 10+ messages in thread From: Uday S Reddy @ 2011-03-13 16:24 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: message body text --] [-- Type: text/plain, Size: 378 bytes --] The debate on save-excursion getting "defeated" flared up again on gnu.emacs.help. Somebody noticed that the Lisp manual hasn't been updated properly to take account of the new compiler warning. The attached patch/bundle is an attempt to explain the situation as well as to provide guidance on how to use these forms correctly. Please let me have any comments. Cheers, Uday [-- Attachment #2: save-excursion and save-current-buffer - edits to the manual --] [-- Type: text/plain, Size: 7482 bytes --] # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: u.s.reddy@cs.bham.ac.uk-20110313160841-rowr3yt2r3iv5s5x # target_branch: file:///D:/gnu/emacs-23-dev/ # testament_sha1: 2bb9efa1bb4c2102b3bb3a153847419b08046134 # timestamp: 2011-03-13 16:09:13 +0000 # base_revision_id: lekktu@gmail.com-20110312194442-ujp5i3ojwxqo62uv # # Begin patch === modified file 'doc/lispref/buffers.texi' --- doc/lispref/buffers.texi 2011-01-02 23:50:46 +0000 +++ doc/lispref/buffers.texi 2011-03-13 16:08:41 +0000 @@ -113,8 +113,8 @@ as well as from the command loop; it is convenient for the caller if the subroutine does not change which buffer is current (unless, of course, that is the subroutine's purpose). Therefore, you should -normally use @code{set-buffer} within a @code{save-current-buffer} or -@code{save-excursion} (@pxref{Excursions}) form that will restore the +normally use @code{set-buffer} within a @code{save-current-buffer} +that will restore the current buffer when your function is done. Here, as an example, is a simplified version of the command @code{append-to-buffer}: @@ -149,8 +149,8 @@ binding's scope. Otherwise you might bind it in one buffer and unbind it in another! There are two ways to do this. In simple cases, you may see that nothing ever changes the current buffer within the scope of the -binding. Otherwise, use @code{save-current-buffer} or -@code{save-excursion} to make sure that the buffer current at the +binding. Otherwise, use @code{save-current-buffer} +to make sure that the buffer current at the beginning is current again whenever the variable is unbound. Do not rely on using @code{set-buffer} to change the current buffer @@ -171,6 +171,36 @@ Using @code{save-current-buffer}, as we did, handles quitting, errors, and @code{throw}, as well as ordinary evaluation. +Sometimes the @code{save-excursion} (@pxref{Excursions}) form is used +instead of @code{save-current-buffer} to restore the current-buffer. +@code{save-excursion} restores the current buffer and, in addition, +restores the point and mark in that buffer as well. However, it is +not a recommended use of @code{save-excursion} for reverting buffer +changes. If you use it, the byte compiler gives the somewhat +mysterious warning: + +@example +Warning: @code{save-excursion} defeated by @code{set-buffer} +@end example + +@noindent The reasons for the warning are: + +@itemize +@item +The @code{save-excursion} only restores the original buffer's point and +mark, not in the target of the @code{set-buffer}. +@item +If the body of the @code{save-excursion} somehow changes the original +buffer's point and mark, then it is best to place +@code{save-excursion} around the portion of the code that does so. +@end itemize + +@noindent Replacing @code{save-excursion} by +@code{save-current-buffer} is always a good way to get around the +compiler warning. However, before you can do the replacement, you +will need to ensure that all the changes to the point and mark inside +the body are properly protected by @code{save-excursion} forms. + @defun current-buffer This function returns the current buffer. === modified file 'doc/lispref/positions.texi' --- doc/lispref/positions.texi 2011-01-02 23:50:46 +0000 +++ doc/lispref/positions.texi 2011-03-13 16:08:41 +0000 @@ -808,7 +808,7 @@ described elsewhere (see @ref{Window Configurations}, and @pxref{Frame Configurations}). When only the identity of the current buffer needs to be saved and restored, it is preferable to use -@code{save-current-buffer} instead. +@code{save-current-buffer} instead. (see @ref{Current Buffer}). @defspec save-excursion body@dots{} @cindex mark excursion @@ -867,6 +867,25 @@ @code{deactivate-mark}, and thus causing the deactivation of the mark after the command finishes. @xref{The Mark}. + It is recommended practice to place @code{save-excursion} as close +as possible to the portions of code that change the point or mark. In +particular, there should be no calls to @code{set-buffer} in the +intervening code inside the @code{save-excursion} form before the +changes to point or mark, because @code{save-excursion} only restores the point +and mark of the current buffer, not for the other buffers that may be +the target of @code{set-buffer}. If a @code{save-excursion} form +begins with a call to @code{set-buffer}, the byte compiler +produces the warning: + +@example +Warning: @code{save-excursion} defeated by @code{set-buffer} +@end example + +@noindent You may replace the @code{save-excursion} forms by +@code{save-current-buffer} forms to get around the warning, but you +would also need to ensure that the changes to point and mark in the +target buffers are protected by @code{save-excursion} forms locally. + @node Narrowing @section Narrowing @cindex narrowing # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWQDx/gEAAzLfgAAQUOf/93pg nqC////6YAhnPuV2dcUpN1hHdu+QaG2L205ISiIaT1MFMxBNNPU9TIHqZADQzUNBKIAEaBTySBpt QAAADQA5gE0wCZDAAEwTAAABIkCp6aTQ9GlP0k9NEZPSaDQDTIDTQRSmknqeUeoPQnpNBpjU0GgA AABIkE0CaAmk8k9TNKemiHqGQaNNGgjSlLkoqyugc0cz2dVeV48LvepPxVlk4ixCtYGDLVNulxRn BYHfLxCDerEKKDm54JGSztq9Xsc2ebhLNpyOfWot4zV5hcLZwV1a+Y7jSNtDZxfgNM7ZQdWyJOqi jlk9hS060bMdvdj3s8OklOZQaZ6Ybm6HGTNSX0lZmm3z9k5vpKJMPZdPVPk3SKXc+jtrL7iOsF9a 4XhcbdnfP+PHStm5LKlTLUpMwIEHUYeH86tCRXKmcYLRataqmr8XxMK3ahYnp1m0Z1JMjQyZa9xp BMLFRs1Y4iFGEGDlrlLOuEy+9aRfZ1gMCoRmyjMnSZDlJI/EWzXq9hRSzZsGztB7tKspUV5FzqMM FZF9DoYUexjWRrHG2GWzXYthrNPUWt2INVAoKUfOVZiHRvglTbLbG01hY5qwHDsFckrJZMQOOngL qB9nP1HFYy2d76b+G7pK26efdwQUQXLgQgaBgeSDkbeWREZYbDPreNwlDlomKY0J4btshkyEijgX kMj6EiRocgKimaF4WT6RGYZjUiQGF+XN547DEQ90+DPCZFBOoSQkXXGEZQujpeC99nXuSAxEZ4g8 zP1THWcrGIyoHDhoOwtvzdwJgkImRwx0CRtJG+tEBTERTAhDaYWJUNyZMeb2LZ9eRbZmLA9oDWJs MtVrlRIYk45HKhSeFaGhU1J4GyKBS22chvGAcLcB6IcjAuI1NSqQqPIGk2DppEzFenCUYVqTo6pU gPMHjFNIb7VaVhXVbYVPqLkv8A9ouoFK7eMtxmB0frFmTMFM3FYODTk8KHTr0VmTKMarTpKUB6XM aCZb3K8rHJH7GgHV3TWFBaayLWzzSGmhdIurNglxM1M2pYk0qbjApcZFqVMR58+HkVZ0MbsQd+Gc a25wtklWAddCKqyLrPX6Lf2YaDe1N+I93wOti9MbBNgdSCEb11kC+VFeSGNvukvKfnlJqjSY5QRP jFnVDVsgCFBgnSaDIOz5OOYuQNVqHbXc8wk7OgnxRIsGk2dN3WUcGKCpr9wUuqjJlhcGGE1glmUq AfBH1Kqrve7C5ok1Eh5bEC8ZGB9vYT5XMt3bCPcVzPNrR80EdsjgeyETf3jPPzbkhS9BMCgjUg9D qpkPXPaNZ6HftnHp4VvWTdSUkfJCW9ZG6ZddVBqqSZaq2Ni6gieJ4EBA22i6FCatEsr90i/lKhQH WaP7t7fAuziK50+vNzjwGrVQlkdSn7bWWLvL7iBUgaDsxN+MpGuYU7Ik6tQ2+BgdQ1YtSzmg7Bjf ayKDB3R8Y4KBO3qQJwsHMXkRKNwPOc7rdWVKoYvMZxOO3ZxAyRQxWaSC3xIGMgEd5WZyYTbWsWBC WQmGi04KUtmetu5ht1G/RBlvXCYc5yGCam0iaYsscGXOG2UGrbb69kdaid0qTEqcssvW1bXIO+nJ 1FNFkpbyYo2m4yiGyrPYi7D8xbDNBEwSMhluOMzYNI20wwXDMK0WvOK+5oMJohpITNARRInTEQtI wlSSoUlI7URx0OQu5BAuc0B9WJDYGbRXDUB5iiZvUjOs40j1/6ihEuaqty7BdVrKrd1aPA96PBcd /Hbe1owteVdgI/Xn2yvN7GJtg0BusjX9iPaiS1dxDoSuDy6ga4JFu9bFuzSkA4j1c0dF2qRYG+9T DBEoDmi17ICpyYcU1v+J8TPMZm2RmZa/PJvj2kjQXTbMFVBqJtXUaV7hMoV3l2etzHOZJUooUBcF pHnZRS6JA0mXk1NNGic0SGEykApi776sVCvJYEHuOYqLBgwsREIaGmME2mw0MZKYB4dA3sXqaXrj WL5KRxdu0ANxzKlNqDFeoDorLL6OBa1MOqTyVL9bOWNiRBEB3EiTXC1t5smSdoK4d6/zy31WiPEk B0eSzehjDdNZxqZuUBoJCOLx8W5dX2owjFnXoYC968DUloeLhjFDR8gOIGyiK1K6KYewnmuiCyNW XYhVSqWBo2PZEKRNTMvOUyUAOEiEumNGEl5AaIVwmCJQXJjTEtvM+F8tdcOviTpOdWB0WhUzWt01 rCDFBN6iir6QM+50dgm+rACut0MoDPVfhskRhNeCNrmsxQxBkFTQOnC9cLI2WZgcc0SE5XNXDRWI RQZDiCRY9kplCFynFGFqKo02mDvA5CsiwW4ZHwLFKGZwLzJG2mPaBiLbWEs2kNoQ2LB/9PCePYbN 5Eh1bCyC9pZpGYUYU9FUmUZzOXMGv1329jLg+IRIhxtqF+s0w2B/E6rLBbCU8rhAzjeQcji23G6k JVUO/ExLelRBkKVAN4ves0rC+YwCGB3L3QleXMNcPKM5M4RE25oNQT8/yAsS2IxMcDEhCWIbUhts BBbAf4u5IpwoSAB4/wCA ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: save-excursion and save-current-buffer - edits to the manual 2011-03-13 16:24 save-excursion and save-current-buffer - edits to the manual Uday S Reddy @ 2011-03-13 18:27 ` Drew Adams 2011-03-13 21:22 ` Uday S Reddy 2011-03-13 21:40 ` Stefan Monnier 2011-03-19 20:36 ` Chong Yidong 1 sibling, 2 replies; 10+ messages in thread From: Drew Adams @ 2011-03-13 18:27 UTC (permalink / raw) To: 'Uday S Reddy', emacs-devel > The debate on save-excursion getting "defeated" flared up again on > gnu.emacs.help. Somebody noticed that the Lisp manual hasn't been > updated properly to take account of the new compiler warning. No, someone simply pointed out that the Lisp manual does not say what you were trying to make it say. What it says is correct. It just hasn't yet been purged of `save-excursion' the way you would like. And that's a good thing. > The attached patch/bundle is an attempt to explain the situation as > well as to provide guidance on how to use these forms correctly. > Please let me have any comments. My comment is to please leave it as it was. You have replaced mention of using `save-excursion' or `save-current-buffer' when one uses `set-buffer' with just mention of using `save-current-buffer' with it. Both can be useful here. You've added this: "However, it is not a recommended use of `save-excursion' for reverting buffer changes." The word "it" references nothing here - no meaning. And `save-excursion' is not about reverting buffer changes - never has been. And no one would ever think that it is. And the warning you then try to explain also has nothing to do with reverting buffer changes. And your explanation of the warning, like the warning itself, does not help. And in your "recommended practice" section, you completely miss that `save-excursion' is not only about restoring point (and mark). It is specifically designed to restore `current-buffer' as well. And there should be no "there should be"s in technical doc: "there should be no calls to `set-buffer' in the intervening code inside the `save-excursion' form before the changes to point or mark," Nonsense. Let's help users understand, not just give them a catechism to follow blindly. "because `save-excursion' only restores the point and mark of the current buffer, not for the other buffers that may be the target of `set-buffer'" Doesn't follow at all. That certainly is not a reason not to use `set-buffer' in the body of `save-excursion' before any point movements. No connection. In your "because A", A is true - so what? It does not follow that you should not move point in buffer X after calling (set-buffer X). This is getting sillier and sillier. All because of a warning that no one can decipher and the explanations for which go round and round...nowhere. This is how we end up with "guidance" in the manual that is later pointed to as proof, The Word, and is hammered into user heads as a catechism. This is obviously a controversial area where judgments differ and programmer judgment is called for. Please do not try to carve one view of this in stone. Just give users the facts of what these functions do, and let them decide for themselves whether and how to use them. Users are not idiots, in general. Underlying your attempt to revise the manual here, your real contribution is in underlining that this message is incomprehensible and unhelpful on its own. The right solution is to get rid of the misguided message, IMO. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-13 18:27 ` Drew Adams @ 2011-03-13 21:22 ` Uday S Reddy 2011-03-14 0:26 ` Drew Adams 2011-03-13 21:40 ` Stefan Monnier 1 sibling, 1 reply; 10+ messages in thread From: Uday S Reddy @ 2011-03-13 21:22 UTC (permalink / raw) To: emacs-devel On 3/13/2011 6:27 PM, Drew Adams wrote: >> The debate on save-excursion getting "defeated" flared up again on >> gnu.emacs.help. Somebody noticed that the Lisp manual hasn't been >> updated properly to take account of the new compiler warning. > > No, someone simply pointed out that the Lisp manual does not say what you were > trying to make it say. What it says is correct. It just hasn't yet been purged > of `save-excursion' the way you would like. And that's a good thing. I wasn't trying to settle the dispute in the manual, which is not the place for it anyway. The point is merely to make it faithful to the compiler and put enough helpful text for people that would be mystified by the new compiler warning. The Current Buffer section has not been updated after the compiler warning was introduced. So, it does need revision. In particular, it contradicts the compiler warning by saying explicitly that one can use save-excursion whereas the compiler says no. >> The attached patch/bundle is an attempt to explain the situation as >> well as to provide guidance on how to use these forms correctly. >> Please let me have any comments. > > My comment is to please leave it as it was. We can't, because it contradicts the compiler. > You have replaced mention of using `save-excursion' or `save-current-buffer' > when one uses `set-buffer' with just mention of using `save-current-buffer' with > it. Both can be useful here. No, the compiler complains about save-excursion being used for this purpose. > You've added this: > > "However, it is not a recommended use of `save-excursion' for reverting buffer > changes." > > The word "it" references nothing here - no meaning. If you really want "it" to refer to something, think of "it" as the compiler. > And `save-excursion' is not about reverting buffer changes - never has been. > And no one would ever think that it is. And the warning you then try to explain > also has nothing to do with reverting buffer changes. But this section is about buffer changes. And, in any case, the sentence before the one you quoted says: "save-excursion restores the current buffer and, in addition, restores the point and mark in that buffer as well." > And your explanation of the warning, like the warning itself, does not help. Since you think the warning doesn't help, I am not surprised that you don't find the explanation helpful. But, perhaps, you can be a bit more objective here? In what way does it not help? > And in your "recommended practice" section, you completely miss that > `save-excursion' is not only about restoring point (and mark). It is > specifically designed to restore `current-buffer' as well. > > And there should be no "there should be"s in technical doc: > > "there should be no calls to `set-buffer' in the intervening code > inside the `save-excursion' form before the changes to point or mark," > > Nonsense. Let's help users understand, not just give them a catechism to follow > blindly. > > "because `save-excursion' only restores the point and mark of > the current buffer, not for the other buffers that may be the target > of `set-buffer'" > > Doesn't follow at all. It does follow. Here is the logic, spelled out: 1. save-excursion should be placed as close as possible to the point/mark-movements. 2. save-excursion only restores the current buffer's point/mark. So, if there is a set-buffer between the save-excursion and the point/mark-movements, then it is not as close as possible to the point/mark-movements. (In fact, in the normal cases where the set-buffer's are not no-op's , there would have to be at least *two* set-buffer's between the save-excursion and the point/mark-movements that it is reverting.) > This is getting sillier and sillier. All because of a warning that no one can > decipher and the explanations for which go round and round...nowhere. Sorry, I have been able to decipher it just fine. On the other hand, I haven't been able to decipher what your objection is about. So, we could perhaps make some progress if you can help me understand your objection. If it is just a political position on what compilers should or should not do, I couldn't be interested because I didn't write the compiler. But if there is a technical point, what is it? Cheers, Uday ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: save-excursion and save-current-buffer - edits to the manual 2011-03-13 21:22 ` Uday S Reddy @ 2011-03-14 0:26 ` Drew Adams 2011-03-14 1:17 ` Tim Cross 2011-03-14 22:33 ` Richard Stallman 0 siblings, 2 replies; 10+ messages in thread From: Drew Adams @ 2011-03-14 0:26 UTC (permalink / raw) To: 'Uday S Reddy', emacs-devel > The point is merely to make it faithful to the compiler... > ... it contradicts the compiler warning by saying explicitly that > one can use save-excursion whereas the compiler says no. > > > My comment is to please leave it as it was. > > We can't, because it contradicts the compiler. > > > Both [functions] can be useful here. > > No, the compiler complains about save-excursion being used > for this purpose. So now we've apparently gone all the way - from a warning issued supposedly just in case, because a tiny minority of cases where `save-excursion' is used with `set-buffer' are deemed (by some) to be inappropriate or not ideal, to your claiming outright that use of `save-excursion' "contradicts the compiler" and "the compiler says no", it simply cannot be used. This is exactly how we go (quickly, it seems) down the road from carefully reasoned judgment calls to black & white catechism. Sheesh. Those who come later will have no inkling of understanding (certainly not from your addition to the manual), but will no doubt feel proud to parrot "The Rule". Sorry, but this is a disservice. You yourself have indicated that of the zillions of cases the compiler flagged for you in the code you inherited, you changed only a small minority in the end. So certainly it is not a no-no to use `save-excursion' with `set-buffer'. Even by your own estimation of what was needed in your code. Sorry, but both `save-excursion' and `save-current-buffer' _can_ be useful here. And it is simply a bad idea to purge the manual this way. Regardless of what the compiler has been made to say or suggest recently. The manual will hopefully offer more understanding about this than the compiler does! And the manual does so already, in the doc of `save-excursion'. It is very clear, from both the manual and the doc string, what `save-excursion' does. But one has to take the trouble to read it. What is in the body of `save-excursion' is irrelevant to what `save-excursion' does. The function simply makes the original buffer, point, and mark current again when it ends. To understand that is to understand everything about it, including how and when to use it. > > "However, it is not a recommended use of `save-excursion' > > for reverting buffer changes." > > > > The word "it" references nothing here - no meaning. > > If you really want "it" to refer to something, think of "it" as the > compiler. What do _you_ really want? "However, _the compiler_ is not a recommended use of `save-excursion' for reverting buffer changes"? From awful to worse... > > And `save-excursion' is not about reverting buffer changes > > - never has been. And no one would ever think that it is. > > And the warning you then try to explain also has nothing to do > > with reverting buffer changes. > > But this section is about buffer changes. And, in any case, the > sentence before the one you quoted says: > > "save-excursion restores the current buffer and, in addition, > restores the point and mark in that buffer as well." Yes, but none of those things have anything to do with changing the content of a buffer, which is what _reverting buffer changes_ refers to. Changing which buffer is current is not related to reverting buffer changes. Nor is changing point or mark. It is only your proposed addition that introduces "reverting buffer changes". > > And your explanation of the warning, like the warning > > itself, does not help. > > Since you think the warning doesn't help, Believe me, I'm not the only one who thinks that. Not that it matters... > I am not surprised that you don't find the explanation helpful. > But, perhaps, you can be a bit more objective here? In what way > does it not help? See above. You give the distinct impression that it is _wrong_ to use `save-excursion' with `set-buffer'. It is not wrong. That mistaken impression does not help. > > "there should be no calls to `set-buffer' in the intervening code > > inside the `save-excursion' form before the changes to > > point or mark, because `save-excursion' only restores the point > > and mark of the current buffer, not for the other buffers that > > may be the target of `set-buffer'" > > > > Doesn't follow at all. > > It does follow. Here is the logic, spelled out: No, here is _your_ logic spelled out - your logic and your words: 1. "`save-excursion' only restores the point and mark of the current buffer, not for other buffers that may be the target of `set-buffer'." 2. THEREFORE, do not call `set-buffer' inside `save-excursion' and then move point or mark. Look at the quotation just above this to see if I'm not right. That's just what you said: #1 implies #2; #2 is "because" of #1. #2 simply does _not_ follow from #1. No way, no how. #1 just says what `save-excursion' does, nothing more. No objection. #2 is your rule, parachuted in from nowhere. Nothing in #1 implies #2. > 1. save-excursion should be placed as close as possible to the > point/mark-movements. > 2. save-excursion only restores the current buffer's point/mark. > > So, if there is a set-buffer between the save-excursion and the > point/mark-movements, then it is not as close as possible to the > point/mark-movements. Huh? First, you are conflating things, bringing in a general principle of software engineering that no one disagrees with and that does not apply here specially: do not needlessly extend scope. More importantly, changing point and mark after invoking `set-buffer' changes them in another buffer, the target of `set-buffer'. `save-excursion' has _no effect_ whatsoever on point and mark in that buffer (unless it is also the orginal buffer). Surely that should be clear by now. It is a good idea (but not cause for any warning) to wrap a construct such as `save-excursion' as tightly as possible around the things it is intended to undo/restore. No one argues the contrary. If the `set-buffer' is to be reversed, i.e., the original buffer is to be restored after the `set-buffer', then the scope must extend beyond the `set-buffer', but certainly not because of any point or mark movements in that buffer - `save-excursion' has no effect on them. (Unless of course it is the same buffer.) And if there is code after the `set-buffer' that is also in the category of changes to be reversed/restored, then the scope should be closed only after that as well. If not, then its scope should be ended before that. But again, `save-excursion' cannot restore anything more than (a) which buffer was originally current, (b) its point, and (c) its mark. At any rate, none of this is what the warning is about, according to the various defenses of it that have been presented. The warning does not say "The scope of `save-excursion' is looser than it needs to be in this occurrence". If it did, and it were accurate, I would likely be in favor of it (but still not as a _warning_). > (In fact, in the normal cases where the > set-buffer's are not no-op's , there would have to be at least *two* > set-buffer's between the save-excursion and the point/mark-movements > that it is reverting.) Huh? I have no idea what you are talking about now. `save-excursion' does not revert point/mark in any buffers except the original one. How many `set-buffer's there are and which buffers become current at different points in the body are irrelevant. And that includes any `set-buffer's back to the original buffer. The only point movements in the body of a `save-excursion' that are affected (rolled back) are those in the orginal buffer. `save-excursion' always makes the original buffer current again and always restores its original point and mark. Only. Nothing more. What you do in the body of the `save-excursion' has no effect on this restoration action (well, you could kill the original buffer...), and `save-excursion' has no other action. None. > > This is getting sillier and sillier. All because of a > > warning that no one can decipher and the explanations for > > which go round and round...nowhere. > > Sorry, I have been able to decipher it just fine. Yeah, and there are those who claim to hear directly from Ms Supreme Being and impart Her wisdom to us mortels who cannot hear Her. But each such prophet hears a different story - go figure. Kool Aid. I'm sure you're deciphering. I'm not sure you're deciphering the same thing as others (there have been several different explanations/interpretations of what is being warned about, including a few by Stefan himself). But even if you are deciphering exactly what Stefan intends that doesn't make the warning a good one. It doesn't even make the idea of trying to have such a warning a good idea. The warning idea is entirely misguided, IMHO. What's called for is making clear what `save-excursion' does and does not do. That's all. Help users understand what really happens and it will be clear enough to them how and when to use it. Will there be zero mistakes made then? No. But there is nothing to warn about, nothing at all. > On the other hand, I haven't been able to decipher what your > objection is about. And yet I've been explicit. Let me repeat it for you: 1. I object to having the warning at all (annoying, misleading). 2. I object to the warning's being a WARNING rather than an informative msg. There is no DANGER here. 3. I object to the warning's not doing any good: it is unable to target real or even possible problems narrowly. Catching all occurrences of `save-excursion' with `set-buffer' indicts far more innocent than guilty cases, even by Stefan's (and your own previous) judgment. It is a blanket alarm. 4. I object to the warning's doing real harm: (a) It and you seem to claim that using `save-excursion' with `set-buffer' is always or even usually a bad idea. And that's just not true. (b) It scares users when they byte-compile a library, and scares them only abstractly, without imparting any understanding of what the supposed dangers are - it's just a big, wild, noisy, scary scream: DANGER!!! LOCK THE DOOR! (c) Overuse of WARNINGs for things that are not dangerous dilutes the meaning of warnings - ask Chicken Little and the Boy Who Cried "Wolf!". What would you think of a loud speaker and flashing red lights in your car announcing every 30 seconds while you drive, "ATTENTION!! DRIVING CAN BE DANGEROUS"? Think it's helpful? And yet in that case there is a _real danger_! 5. I object to your interepreting the problem, in the manual, as being about placing `save-excursion' closer to point movements (regardless of which buffer they are in). That is not the crux of the problems cited by Stefan or others. That's just sane coding practice - it applies equally to `let', and it certainly has nothing to do with a warning or danger. And the warning does not reflect scope tightness; it is a knee-jerk alarm. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-14 0:26 ` Drew Adams @ 2011-03-14 1:17 ` Tim Cross 2011-03-14 22:33 ` Richard Stallman 1 sibling, 0 replies; 10+ messages in thread From: Tim Cross @ 2011-03-14 1:17 UTC (permalink / raw) To: Drew Adams; +Cc: Uday S Reddy, emacs-devel [-- Attachment #1: Type: text/plain, Size: 13518 bytes --] On Mon, Mar 14, 2011 at 11:26 AM, Drew Adams <drew.adams@oracle.com> wrote: > > The point is merely to make it faithful to the compiler... > > ... it contradicts the compiler warning by saying explicitly that > > one can use save-excursion whereas the compiler says no. > > > > > My comment is to please leave it as it was. > > > > We can't, because it contradicts the compiler. > > > > > Both [functions] can be useful here. > > > > No, the compiler complains about save-excursion being used > > for this purpose. > > So now we've apparently gone all the way - from a warning issued supposedly > just > in case, because a tiny minority of cases where `save-excursion' is used > with > `set-buffer' are deemed (by some) to be inappropriate or not ideal, to your > claiming outright that use of `save-excursion' "contradicts the compiler" > and > "the compiler says no", it simply cannot be used. > > This is exactly how we go (quickly, it seems) down the road from carefully > reasoned judgment calls to black & white catechism. Sheesh. Those who > come > later will have no inkling of understanding (certainly not from your > addition to > the manual), but will no doubt feel proud to parrot "The Rule". Sorry, but > this > is a disservice. > > You yourself have indicated that of the zillions of cases the compiler > flagged > for you in the code you inherited, you changed only a small minority in the > end. > So certainly it is not a no-no to use `save-excursion' with `set-buffer'. > Even > by your own estimation of what was needed in your code. > > Sorry, but both `save-excursion' and `save-current-buffer' _can_ be useful > here. > And it is simply a bad idea to purge the manual this way. Regardless of > what > the compiler has been made to say or suggest recently. The manual will > hopefully offer more understanding about this than the compiler does! > > And the manual does so already, in the doc of `save-excursion'. It is very > clear, from both the manual and the doc string, what `save-excursion' does. > But > one has to take the trouble to read it. > > What is in the body of `save-excursion' is irrelevant to what > `save-excursion' > does. The function simply makes the original buffer, point, and mark > current > again when it ends. To understand that is to understand everything about > it, > including how and when to use it. > > > > "However, it is not a recommended use of `save-excursion' > > > for reverting buffer changes." > > > > > > The word "it" references nothing here - no meaning. > > > > If you really want "it" to refer to something, think of "it" as the > > compiler. > > What do _you_ really want? "However, _the compiler_ is not a recommended > use of > `save-excursion' for reverting buffer changes"? From awful to worse... > > > > And `save-excursion' is not about reverting buffer changes > > > - never has been. And no one would ever think that it is. > > > And the warning you then try to explain also has nothing to do > > > with reverting buffer changes. > > > > But this section is about buffer changes. And, in any case, the > > sentence before the one you quoted says: > > > > "save-excursion restores the current buffer and, in addition, > > restores the point and mark in that buffer as well." > > Yes, but none of those things have anything to do with changing the content > of a > buffer, which is what _reverting buffer changes_ refers to. Changing which > buffer is current is not related to reverting buffer changes. Nor is > changing > point or mark. It is only your proposed addition that introduces > "reverting > buffer changes". > > > > And your explanation of the warning, like the warning > > > itself, does not help. > > > > Since you think the warning doesn't help, > > Believe me, I'm not the only one who thinks that. Not that it matters... > > > I am not surprised that you don't find the explanation helpful. > > But, perhaps, you can be a bit more objective here? In what way > > does it not help? > > See above. You give the distinct impression that it is _wrong_ to use > `save-excursion' with `set-buffer'. It is not wrong. That mistaken > impression > does not help. > > > > "there should be no calls to `set-buffer' in the intervening code > > > inside the `save-excursion' form before the changes to > > > point or mark, because `save-excursion' only restores the point > > > and mark of the current buffer, not for the other buffers that > > > may be the target of `set-buffer'" > > > > > > Doesn't follow at all. > > > > It does follow. Here is the logic, spelled out: > > No, here is _your_ logic spelled out - your logic and your words: > > 1. "`save-excursion' only restores the point and mark of the > current buffer, not for other buffers that may be the target > of `set-buffer'." > > 2. THEREFORE, do not call `set-buffer' inside `save-excursion' > and then move point or mark. > > Look at the quotation just above this to see if I'm not right. > That's just what you said: #1 implies #2; #2 is "because" of #1. > > #2 simply does _not_ follow from #1. No way, no how. > > #1 just says what `save-excursion' does, nothing more. No objection. > #2 is your rule, parachuted in from nowhere. Nothing in #1 implies #2. > > > 1. save-excursion should be placed as close as possible to the > > point/mark-movements. > > 2. save-excursion only restores the current buffer's point/mark. > > > > So, if there is a set-buffer between the save-excursion and the > > point/mark-movements, then it is not as close as possible to the > > point/mark-movements. > > Huh? First, you are conflating things, bringing in a general principle of > software engineering that no one disagrees with and that does not apply > here > specially: do not needlessly extend scope. > > More importantly, changing point and mark after invoking `set-buffer' > changes > them in another buffer, the target of `set-buffer'. `save-excursion' has > _no > effect_ whatsoever on point and mark in that buffer (unless it is also the > orginal buffer). Surely that should be clear by now. > > It is a good idea (but not cause for any warning) to wrap a construct such > as > `save-excursion' as tightly as possible around the things it is intended to > undo/restore. No one argues the contrary. > > If the `set-buffer' is to be reversed, i.e., the original buffer is to be > restored after the `set-buffer', then the scope must extend beyond the > `set-buffer', but certainly not because of any point or mark movements in > that > buffer - `save-excursion' has no effect on them. (Unless of course it is > the > same buffer.) > > And if there is code after the `set-buffer' that is also in the category of > changes to be reversed/restored, then the scope should be closed only after > that > as well. If not, then its scope should be ended before that. But again, > `save-excursion' cannot restore anything more than (a) which buffer was > originally current, (b) its point, and (c) its mark. > > At any rate, none of this is what the warning is about, according to the > various > defenses of it that have been presented. The warning does not say "The > scope of > `save-excursion' is looser than it needs to be in this occurrence". If it > did, > and it were accurate, I would likely be in favor of it (but still not as a > _warning_). > > > (In fact, in the normal cases where the > > set-buffer's are not no-op's , there would have to be at least *two* > > set-buffer's between the save-excursion and the point/mark-movements > > that it is reverting.) > > Huh? I have no idea what you are talking about now. `save-excursion' does > not > revert point/mark in any buffers except the original one. How many > `set-buffer's there are and which buffers become current at different > points in > the body are irrelevant. And that includes any `set-buffer's back to the > original buffer. The only point movements in the body of a > `save-excursion' > that are affected (rolled back) are those in the orginal buffer. > > `save-excursion' always makes the original buffer current again and always > restores its original point and mark. Only. Nothing more. > > What you do in the body of the `save-excursion' has no effect on this > restoration action (well, you could kill the original buffer...), and > `save-excursion' has no other action. None. > > > > This is getting sillier and sillier. All because of a > > > warning that no one can decipher and the explanations for > > > which go round and round...nowhere. > > > > Sorry, I have been able to decipher it just fine. > > Yeah, and there are those who claim to hear directly from Ms Supreme Being > and > impart Her wisdom to us mortels who cannot hear Her. But each such prophet > hears a different story - go figure. Kool Aid. > > I'm sure you're deciphering. I'm not sure you're deciphering the same > thing as > others (there have been several different explanations/interpretations of > what > is being warned about, including a few by Stefan himself). > > But even if you are deciphering exactly what Stefan intends that doesn't > make > the warning a good one. It doesn't even make the idea of trying to have > such a > warning a good idea. The warning idea is entirely misguided, IMHO. > > What's called for is making clear what `save-excursion' does and does not > do. > That's all. Help users understand what really happens and it will be clear > enough to them how and when to use it. Will there be zero mistakes made > then? > No. But there is nothing to warn about, nothing at all. > > > On the other hand, I haven't been able to decipher what your > > objection is about. > > And yet I've been explicit. Let me repeat it for you: > > 1. I object to having the warning at all (annoying, misleading). > > 2. I object to the warning's being a WARNING rather than an informative > msg. > There is no DANGER here. > > 3. I object to the warning's not doing any good: it is unable to target > real or > even possible problems narrowly. Catching all occurrences of > `save-excursion' > with `set-buffer' indicts far more innocent than guilty cases, even by > Stefan's > (and your own previous) judgment. It is a blanket alarm. > > 4. I object to the warning's doing real harm: > > (a) It and you seem to claim that using `save-excursion' with `set-buffer' > is > always or even usually a bad idea. And that's just not true. > > (b) It scares users when they byte-compile a library, and scares them only > abstractly, without imparting any understanding of what the supposed > dangers are > - it's just a big, wild, noisy, scary scream: DANGER!!! LOCK THE DOOR! > > (c) Overuse of WARNINGs for things that are not dangerous dilutes the > meaning of > warnings - ask Chicken Little and the Boy Who Cried "Wolf!". > > What would you think of a loud speaker and flashing red lights in your car > announcing every 30 seconds while you drive, "ATTENTION!! DRIVING CAN BE > DANGEROUS"? Think it's helpful? And yet in that case there is a _real > danger_! > > 5. I object to your interepreting the problem, in the manual, as being > about > placing `save-excursion' closer to point movements (regardless of which > buffer > they are in). That is not the crux of the problems cited by Stefan or > others. > That's just sane coding practice - it applies equally to `let', and it > certainly > has nothing to do with a warning or danger. And the warning does not > reflect > scope tightness; it is a knee-jerk alarm. > > > It would seem two different issues are being mixed up here. Issue 1 appears to be a debate concerning whether the situation covered by the warning is serious enough or sufficient to be classified as a warning. At best, I suspect it is borderline - possibly even less so given that it appears to be somewhat 'simplistic' and cannot identify situations where set-buffer within a save-excursion is legit from those where it is either the source of an error or could be replaced with just a with-current-buffer. I do think the information can be useful and would have no issue with it being reported as a notice or even removed and only included in something like elint. The distinction between notice and warning is blurred and changing to notice may not actually help - I tend to think of a notice as more like information rather than a warning of a possible problem. Whether it is a warning, a notice or removed altogether, is largely irrelevant from my standpoint. Issue 2. is, given that the compiler does report this warning, is the manual clear enough or detailed enough that the programmer can understand what the warning means and when to be concerned or not. I think this is the real issue that needs to be addressed. I've not read Uday's suggested contribution and therefore won't comment on it. However, as things currently stand, I don't think the manual addresses the matter sufficiently and that without this, the warning will continue to be a source of confusion (and probably future debate). Either the warning should be removed (which seems unlikely) or we try to enhance the manual to explain what the warning means and provide the background information that would prevent confusion and possibly make the warning more useful. Lets separate the two issues and deal with them individually. Those who want to debate the merits/validity of the warning can do so. Those who want the warning to be less confusing and possibly more informative might like to try and suggest patches to the manual. Tim [-- Attachment #2: Type: text/html, Size: 15581 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-14 0:26 ` Drew Adams 2011-03-14 1:17 ` Tim Cross @ 2011-03-14 22:33 ` Richard Stallman 2011-03-15 7:08 ` Uday S Reddy 1 sibling, 1 reply; 10+ messages in thread From: Richard Stallman @ 2011-03-14 22:33 UTC (permalink / raw) To: Drew Adams; +Cc: u.s.reddy, emacs-devel So now we've apparently gone all the way - from a warning issued supposedly just in case, because a tiny minority of cases where `save-excursion' is used with `set-buffer' are deemed (by some) to be inappropriate or not ideal, to your claiming outright that use of `save-excursion' "contradicts the compiler" and "the compiler says no", it simply cannot be used. Compiler warnings should be programmers' assistants, not their masters. That is why we have with-no-warnings. We implement helpful warnings about constructions that _might_ be mistakes, but that doesn't mean they are taboo. -- Dr Richard Stallman President, Free Software Foundation 51 Franklin St Boston MA 02110 USA www.fsf.org, www.gnu.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-14 22:33 ` Richard Stallman @ 2011-03-15 7:08 ` Uday S Reddy 0 siblings, 0 replies; 10+ messages in thread From: Uday S Reddy @ 2011-03-15 7:08 UTC (permalink / raw) To: rms; +Cc: emacs-devel Richard Stallman writes: > Compiler warnings should be programmers' assistants, not their > masters. That is why we have with-no-warnings. We implement > helpful warnings about constructions that _might_ be mistakes, but > that doesn't mean they are taboo. Of course, the manuals never stop the programmers from doing what they want to do. They can at best explain and advise, and leave the final judgement to the programmer. But we need a coherent story to tell between the compiler and the manual. If the comiler warns "there is a problem doing this" and the manual says "go ahead and do it", then we don't have a coherent picture. Cheers, Uday ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-13 18:27 ` Drew Adams 2011-03-13 21:22 ` Uday S Reddy @ 2011-03-13 21:40 ` Stefan Monnier 2011-03-14 0:45 ` Drew Adams 1 sibling, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2011-03-13 21:40 UTC (permalink / raw) To: Drew Adams; +Cc: 'Uday S Reddy', emacs-devel > The right solution is to get rid of the misguided message, IMO. If you want your messages to be useful, better start by forgetting about this option. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: save-excursion and save-current-buffer - edits to the manual 2011-03-13 21:40 ` Stefan Monnier @ 2011-03-14 0:45 ` Drew Adams 0 siblings, 0 replies; 10+ messages in thread From: Drew Adams @ 2011-03-14 0:45 UTC (permalink / raw) To: 'Stefan Monnier'; +Cc: 'Uday S Reddy', emacs-devel > > The right solution is to get rid of the misguided message, IMO. > > If you want your messages to be useful, better start by > forgetting about this option. As I said earlier, I made my peace with this misguided message long ago, knowing your intention to keep it. Nevertheless, it is MHO that it should be removed, and I haven't been convinced otherwise. I think my messages about this are generally thought out and useful, even if they don't persuade you in particular. Just because someone expresses disagreement, that does not make his expression useless. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: save-excursion and save-current-buffer - edits to the manual 2011-03-13 16:24 save-excursion and save-current-buffer - edits to the manual Uday S Reddy 2011-03-13 18:27 ` Drew Adams @ 2011-03-19 20:36 ` Chong Yidong 1 sibling, 0 replies; 10+ messages in thread From: Chong Yidong @ 2011-03-19 20:36 UTC (permalink / raw) To: Uday S Reddy; +Cc: emacs-devel Uday S Reddy <u.s.reddy@cs.bham.ac.uk> writes: > The debate on save-excursion getting "defeated" flared up again on > gnu.emacs.help. Somebody noticed that the Lisp manual hasn't been > updated properly to take account of the new compiler warning. > > The attached patch/bundle is an attempt to explain the situation as > well as to provide guidance on how to use these forms correctly. > Please let me have any comments. Thank you. I've committed a patch, along the same lines but using different wording, to the branch. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-03-19 20:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-13 16:24 save-excursion and save-current-buffer - edits to the manual Uday S Reddy 2011-03-13 18:27 ` Drew Adams 2011-03-13 21:22 ` Uday S Reddy 2011-03-14 0:26 ` Drew Adams 2011-03-14 1:17 ` Tim Cross 2011-03-14 22:33 ` Richard Stallman 2011-03-15 7:08 ` Uday S Reddy 2011-03-13 21:40 ` Stefan Monnier 2011-03-14 0:45 ` Drew Adams 2011-03-19 20:36 ` Chong Yidong
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).