Patch for bug #6515: unneeded let-bound variables in bookmark.el

Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Christoph]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

Hi

Find attached a bugfix for bug #6515 
(http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515) for review.

Imho, the bug report is valid. I don't think the code in question should 
override globally set variable bookmark-automatically-show-annotations. 
Therefore, the let variables were removed.

I sent in my signed paperwork a while ago, but haven't heard anything 
back, so I hope this is minor enough to maybe go through without it.

Christoph

[-- Attachment #2: bugfix_6515.txt.txt --]
[-- Type: text/plain, Size: 3575 bytes --]

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: cschol2112@gmail.com-20100626175327-syboc05tlvbvcjwg
# target_branch: http://bzr.savannah.gnu.org/r/emacs/trunk/
# testament_sha1: ed110efd0fe83dca9884a93319ad8d3447b1eb7c
# timestamp: 2010-06-26 11:57:42 -0600
# base_revision_id: eliz@gnu.org-20100626144112-032airiym9h9hy1p
# 
# Begin patch
=== modified file 'lisp/bookmark.el'
--- lisp/bookmark.el	2010-04-14 15:07:53 +0000
+++ lisp/bookmark.el	2010-06-26 17:53:27 +0000
@@ -1860,8 +1860,7 @@
         (pop-up-windows t))
     (delete-other-windows)
     (switch-to-buffer (other-buffer))
-    (let ((bookmark-automatically-show-annotations nil)) ;FIXME: needed?
-      (bookmark--jump-via bmrk 'pop-to-buffer))
+    (bookmark--jump-via bmrk 'pop-to-buffer)
     (bury-buffer menu)))
 
 
@@ -1875,8 +1874,7 @@
   "Select this line's bookmark in other window, leaving bookmark menu visible."
   (interactive)
   (let ((bookmark (bookmark-bmenu-bookmark)))
-    (let ((bookmark-automatically-show-annotations t)) ;FIXME: needed?
-      (bookmark--jump-via bookmark 'switch-to-buffer-other-window))))
+    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))
 
 
 (defun bookmark-bmenu-switch-other-window ()
@@ -1887,8 +1885,7 @@
         (pop-up-windows t)
         same-window-buffer-names
         same-window-regexps)
-    (let ((bookmark-automatically-show-annotations t)) ;FIXME: needed?
-      (bookmark--jump-via bookmark 'display-buffer))))
+    (bookmark--jump-via bookmark 'display-buffer)))
 
 (defun bookmark-bmenu-other-window-with-mouse (event)
   "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWSskWiYAA1jfgEAQWOf/91oA
CgC////wUAXtDzonbDVnKUIBy4SSTRMRMwAU0zQ0EyI0PUzSBJTUyYRkmENTUYQBoZDQ0ASRAg1M
UepoAAMgGgADDQ0AAAGgAAAABJICJmppkZMmSbURppPU2oBoGcpryRanH54HQeVlN8CTrJZbdKtZ
C5+0UsUkkrRj4MIFJ7SCCwuVMzXSDjsMxs8UviBKTOxVjNo4kHNrEz3Pak7n4PZe/e48VfZmszFh
wkcc2A7w5+7q6WIBxQY6J6+yHvckbAyTTbzBj7gvuWC+ScNXkmZVgGJkvAt/La12Q87LcRiow4CJ
TIVGZlFB6RUgisUwpjAq9KNOLnO0Z9vORIcN4cj3ZINYqyhwCfpuguG7y7KJ9XhUJqDxOSB84LjG
yQoCsSfYSdd/nrUTGttL0q3uRUygm+hIBe2oalnmbivVBBYVhF5IyeqGHHPPXciINPJhMKt0hZNN
bnEqERneNjgMqVgGkKGbCowb2Cfu4oHI5oVGuOsea7rlxaC2ElkLpBGIggWaGxJTwurT2SCg4XW0
/8RrAy4CigVvfaS6u6mCzxUq58C+ES6XQ1lJwumsRanJV6SL6EDnRUcnyaxfdgUool3MjaulyJv7
oKc3Qx4FUwnpmYXIxw56HUsJBMWahQe+ul220VLTBMVCN8WGLJCKXPB4CI0snLmUDMrK0pHO5VEG
J3RfAbk1KiwpLwK4XmKd+bq8MCfDODJBY3IuIrFILzPgeOz6ZRqYlIbFshwb7Re2WmkDdMNfQVbR
hJ6tJ8i+wi9wu2JJ0SitzNju0HiwmIvF7xFq0EcGxxTdhLldAdiOCbiCHLIcUfAusenKenGZIFO2
SwFMUiRs6oxMiaFSThORFy7RTUuxV/vLHKpkGWBfBy0mKajAzHXGYwnjCcESSaARSjIceo8Q7yeo
anezNBjf9mbSVEHCta1FbxhhMfXSoYjuYGI6VvZ4e6y2OLogONMYZPM93eYiXSwfpMuvnyMUshBc
vnMs8YmJUr7hETgJasb+mCLTIkcYJ7Jxj7X2kVhIRxx6jDqJj+LExGPV+scnVgtwjhSKflxxtKJF
bTzLu2Kx3bxB9EovN4USiC0vLxnIYcM77nkFK9KGRhQzmjymLoPIHYR4mL5QKy0s00Lq4Ywk/uiZ
QFmUgvPJEnaZaPZqtp1izcnl9lUBLvhPUnJzIMh1BGl16mszmajy4ypY6m16RTlVbjWWxHTkKeqh
9hj0v+48PouqOgmFgs565noc6siBrE6mwPtAhFATU4XQdKNgqdxbtLXhyEwUBwWnmbjqbPg3wBMM
x+iR7i4TZ+q6IIIo/kBbDoGCbcB5bIAR9EKoStIVvU4UGk6H2J7h5EcgjpEGAnhTalXpj+MCoeb7
ANCuKgOJLSaOJOoFCpBqleKuFOZAwZJfCRx6iY7iBwDxBwA4DwvTBtI8MDaepFcMjY3vEcShLuJ6
A+ExIQeSR0R5zO5eGhUPKFzKQ8MPZVTA5klOAF8RxoPIfMQ0TYVohxNwrxavFticy0WV4pQ4JHuC
HVgiAHuYhNIjqxYWsl9hORB4qWzqx38EYIsSI6DDC/6AdlIJ/rTOo+NROJ7dhDkuSwLoiJG48JCg
T+WHY0UebCJoeWdarzqkWKcmE5aiM1DI1EETBMMVMzHAtAYmVMMw9SBxADnEVocRhxyDEkw+ZU2B
xk8dsVDwFUwQYoJ3sLE7BML6uzr1B5fo3O0/y0rbLwZYC0FsqCquE3NZgXvoKFsxRF6rH3+qccrB
XmjBurBSmuBbBDqjKw/xdyRThQkCskWiYA==

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Christoph]

On 6/26/2010 3:43 PM, I wrote:
> Hi
>
> Find attached a bugfix for bug #6515
> (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515) for review.

Sorry, this was intended to go to the bug tracker.

Christoph

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Karl Fogel]

Christoph <cschol2112@googlemail.com> writes:
>> Find attached a bugfix for bug #6515
>> (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515) for review.
>
>Sorry, this was intended to go to the bug tracker.

Actually, I'm glad you sent it here.  IMHO, the patch is small enough
and trivial enough to go in regardless of assignment papers -- a mere
description of the problem would suffice to cause any reasonable
programmer to generate the exact same patch, if my understanding is
correct (I haven't looked at it in depth yet, but will soon).

Thanks,
-Karl

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Karl Fogel]

Karl Fogel <kfogel@red-bean.com> writes:
>Christoph <cschol2112@googlemail.com> writes:
>>> Find attached a bugfix for bug #6515
>>> (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515) for review.
>>
>>Sorry, this was intended to go to the bug tracker.
>
>Actually, I'm glad you sent it here.  IMHO, the patch is small enough
>and trivial enough to go in regardless of assignment papers -- a mere
>description of the problem would suffice to cause any reasonable
>programmer to generate the exact same patch, if my understanding is
>correct (I haven't looked at it in depth yet, but will soon).

I clean-roomed it (because I wasn't sure it was small enough to not
worry about papers), and of course the result was byte-for-byte the same
as the original patch.  <shrug>

I checked it in, and then immediately saw in

  http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515

that Yidong says he checked it in as well.  But oddly, my trunk (which
is bound to master trunk) doesn't have the change, even after updating.
I'm not sure what's going on with that, but in any case the change is
certainly installed in trunk now, because

  http://bzr.savannah.gnu.org/r/emacs/trunk/.bzr/branch/last-revision

currently says:

  100717 kfogel@red-bean.com-20100704195702-mbiefzxkbx7dgsby

...which is the change I checked in.

Thanks for pointing out this bug!

-Karl

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Christoph]

On 7/4/2010 5:07 PM, Karl Fogel wrote:
> I clean-roomed it (because I wasn't sure it was small enough to not
> worry about papers), and of course the result was byte-for-byte the same
> as the original patch.<shrug>

My paperwork confirmation email arrived the other day, so this shouldn't 
be a problem anymore.

> I checked it in, and then immediately saw in
>
>    http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515
>
> that Yidong says he checked it in as well.  But oddly, my trunk (which
> is bound to master trunk) doesn't have the change, even after updating.
> I'm not sure what's going on with that, but in any case the change is
> certainly installed in trunk now, because

I would guess maybe Chong committed this to the emacs-23 branch? That 
way it is fixed in 23.3 and would get merged into the trunk with the 
next wave of fixes from emacs-23.

Christoph

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Karl Fogel]

Christoph <cschol2112@googlemail.com> writes:
>> I checked it in, and then immediately saw in
>>
>>    http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6515
>>
>> that Yidong says he checked it in as well.  But oddly, my trunk (which
>> is bound to master trunk) doesn't have the change, even after updating.
>> I'm not sure what's going on with that, but in any case the change is
>> certainly installed in trunk now, because
>
>I would guess maybe Chong committed this to the emacs-23 branch? That
>way it is fixed in 23.3 and would get merged into the trunk with the
>next wave of fixes from emacs-23.

In which case I may well have done the Wrong Thing and will cause a
conflict.  We'll see.  One way or another, the patch is in now.  Thanks!

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Glenn Morris]

Karl Fogel wrote:

>>I would guess maybe Chong committed this to the emacs-23 branch? That
>>way it is fixed in 23.3 and would get merged into the trunk with the
>>next wave of fixes from emacs-23.
>
> In which case I may well have done the Wrong Thing and will cause a
> conflict.  We'll see.

Yes, but no dig deal, I guess.

See admin/notes/bzr and the emacs-devel mails referenced therein.

    In particular, install bug-fixes only on the release branch (if
    there is one) and let them get synced to the trunk; do not install
    them by hand on the trunk as well.

(I backported your previous bug fix to the emacs-23 branch.)

Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el

[Karl Fogel]

Glenn Morris <rgm@gnu.org> writes:
>Yes, but no dig deal, I guess.
>
>See admin/notes/bzr and the emacs-devel mails referenced therein.
>
>    In particular, install bug-fixes only on the release branch (if
>    there is one) and let them get synced to the trunk; do not install
>    them by hand on the trunk as well.
>
>(I backported your previous bug fix to the emacs-23 branch.)

Thanks, Glenn.  I'll look around harder for a release branch next time.