* Patch for bug #6515: unneeded let-bound variables in bookmark.el
@ 2010-06-26 21:43 Christoph
2010-06-26 21:46 ` Christoph
0 siblings, 1 reply; 8+ messages in thread
From: Christoph @ 2010-06-26 21:43 UTC (permalink / raw
To: emacs-devel
[-- 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==
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-06-26 21:43 Patch for bug #6515: unneeded let-bound variables in bookmark.el Christoph
@ 2010-06-26 21:46 ` Christoph
2010-06-26 21:50 ` Karl Fogel
0 siblings, 1 reply; 8+ messages in thread
From: Christoph @ 2010-06-26 21:46 UTC (permalink / raw
Cc: emacs-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-06-26 21:46 ` Christoph
@ 2010-06-26 21:50 ` Karl Fogel
2010-07-04 23:07 ` Karl Fogel
0 siblings, 1 reply; 8+ messages in thread
From: Karl Fogel @ 2010-06-26 21:50 UTC (permalink / raw
To: Christoph; +Cc: emacs-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-06-26 21:50 ` Karl Fogel
@ 2010-07-04 23:07 ` Karl Fogel
2010-07-04 23:27 ` Christoph
0 siblings, 1 reply; 8+ messages in thread
From: Karl Fogel @ 2010-07-04 23:07 UTC (permalink / raw
To: Christoph; +Cc: emacs-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-07-04 23:07 ` Karl Fogel
@ 2010-07-04 23:27 ` Christoph
2010-07-04 23:44 ` Karl Fogel
0 siblings, 1 reply; 8+ messages in thread
From: Christoph @ 2010-07-04 23:27 UTC (permalink / raw
To: Karl Fogel; +Cc: emacs-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-07-04 23:27 ` Christoph
@ 2010-07-04 23:44 ` Karl Fogel
2010-07-05 22:48 ` Glenn Morris
0 siblings, 1 reply; 8+ messages in thread
From: Karl Fogel @ 2010-07-04 23:44 UTC (permalink / raw
To: Christoph; +Cc: emacs-devel
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!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-07-04 23:44 ` Karl Fogel
@ 2010-07-05 22:48 ` Glenn Morris
2010-07-06 1:10 ` Karl Fogel
0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2010-07-05 22:48 UTC (permalink / raw
To: Karl Fogel; +Cc: Christoph, emacs-devel
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.)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch for bug #6515: unneeded let-bound variables in bookmark.el
2010-07-05 22:48 ` Glenn Morris
@ 2010-07-06 1:10 ` Karl Fogel
0 siblings, 0 replies; 8+ messages in thread
From: Karl Fogel @ 2010-07-06 1:10 UTC (permalink / raw
To: Glenn Morris; +Cc: Christoph, emacs-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-07-06 1:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-26 21:43 Patch for bug #6515: unneeded let-bound variables in bookmark.el Christoph
2010-06-26 21:46 ` Christoph
2010-06-26 21:50 ` Karl Fogel
2010-07-04 23:07 ` Karl Fogel
2010-07-04 23:27 ` Christoph
2010-07-04 23:44 ` Karl Fogel
2010-07-05 22:48 ` Glenn Morris
2010-07-06 1:10 ` Karl Fogel
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).