GNU bug report logs - #20150
24.4: bookmarks: return from editing annotations (FIX INCLUDED)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 20150 in the body.
You can then email your comments to 20150 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4: bookmarks: return from editing annotations (FIX INCLUDED)
Date: Fri, 20 Mar 2015 03:24:04 -0400

[Message part 1 (text/plain, inline)]

When completing a bookmark annotation edit, the point on the bmenu-list
would move to point-max and the selected window would be wherever the
annotation buffer had been displayed.

The fix restores focus to the bmenu-list window, and restores point to
where it had been prior to editing the annotation.

(defun bookmark-send-edited-annotation ()
  "Use buffer contents as annotation for a bookmark.
Lines beginning with `#' are ignored."
  (interactive)
  (if (not (derived-mode-p 'bookmark-edit-annotation-mode))
      (error "Not in bookmark-edit-annotation-mode"))
  (goto-char (point-min))
  (while (< (point) (point-max))
    (if (looking-at "^#")
        (bookmark-kill-line t)
      (forward-line 1)))
  ;; Take no chances with text properties.
  (let
    ((annotation (buffer-substring-no-properties (point-min) (point-max)))
	 (bookmark-name bookmark-annotation-name)
     (return-line
       (with-current-buffer "*Bookmark List*"
         (line-number-at-pos)))
     (return-column
       (with-current-buffer "*Bookmark List*"
         (current-column))))
    (bookmark-set-annotation bookmark-name annotation)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (kill-buffer (current-buffer))
    (pop-to-buffer (get-buffer "*Bookmark List*"))
    (bookmark-bmenu-list)
    (goto-char (point-min))
    (forward-line (1- return-line))
    (forward-char return-column)))

-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[attachment (text/plain, attachment)]

[signature.asc (application/pgp-signature, attachment)]

Message #8 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Fri, 20 Mar 2015 10:28:58 -0400

> The fix restores focus to the bmenu-list window, and restores point to
> where it had been prior to editing the annotation.

Could you send it as a patch, otherwise I can't really see what's different.


        Stefan

Message #11 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Fri, 20 Mar 2015 12:22:31 -0400

[Message part 1 (text/plain, inline)]

Pleasure.

On 03/20/2015 10:28 AM, Stefan Monnier wrote:
>> The fix restores focus to the bmenu-list window, and restores point to
>> where it had been prior to editing the annotation.
> 
> Could you send it as a patch, otherwise I can't really see what's different.
> 
> 
>         Stefan
> 


-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[emacs_bug_20150.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, attachment)]

Message #14 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Tue, 2 Jul 2019 03:54:25 +0200

[Message part 1 (text/plain, inline)]

Boruch Baum <boruch_baum <at> gmx.com> writes:

> When completing a bookmark annotation edit, the point on the bmenu-list
> would move to point-max and the selected window would be wherever the
> annotation buffer had been displayed.

I can confirm this bug, and suggest the attached fix.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat.patch (application/octet-stream, attachment)]

Message #19 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Tue, 2 Jul 2019 05:23:59 +0200

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:
> > When completing a bookmark annotation edit, the point on the bmenu-list
> > would move to point-max and the selected window would be wherever the
> > annotation buffer had been displayed.
>
> I can confirm this bug, and suggest the attached fix.

Please see this somewhat improved patch instead.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat-2.patch (application/octet-stream, attachment)]

Message #22 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Wed, 3 Jul 2019 22:05:35 +0200

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:
> > > When completing a bookmark annotation edit, the point on the bmenu-list
> > > would move to point-max and the selected window would be wherever the
> > > annotation buffer had been displayed.
> >
> > I can confirm this bug, and suggest the attached fix.
>
> Please see this somewhat improved patch instead.

Now that the test suite for bookmark.el has hit master, I reworked
this to also include tests.  Please find attached an updated patch.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat-3.patch (application/octet-stream, attachment)]

Message #25 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Boruch Baum <boruch_baum <at> gmx.com>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Thu, 4 Jul 2019 21:20:20 +0200

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:
> Now that the test suite for bookmark.el has hit master, I reworked
> this to also include tests.  Please find attached an updated patch.

I thought about this a bit more and realized there is a problem with
the fix: we can of course also edit an annotation when
bookmark-use-annotations is t.  In this case we would not want to
return to the bookmark list.

The fix will therefore have to be a bit more involved to handle both
cases correctly.  I have attached a fourth (and hopefully final) patch
here.

I also ran into a suspected bug in with-current-buffer (Bug#36497).  I
added a FIXME in one of the test cases so we can clean it up once
that's fixed.

This is my first patch with this many changes, and I did my best to
document it thoroughly.  Not sure if we like that or prefer something
more terse.  Comments would be much appreciated.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat-4.patch (application/octet-stream, attachment)]

Message #28 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Thu, 4 Jul 2019 21:40:45 +0200

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:
> I also ran into a suspected bug in with-current-buffer (Bug#36497).  I
> added a FIXME in one of the test cases so we can clean it up once
> that's fixed.

Turns out this was not a bug, but only my own confusion.  Gmail had
helpfully put the reply to the bug report in the spam folder, so I
only realized *after* I sent this email that it had been replied to
hours ago.

I've attached an updated patch without this FIXME.  Other than that,
it's identical.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat-5.patch (application/octet-stream, attachment)]

Message #31 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sat, 13 Jul 2019 10:30:01 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 4 Jul 2019 21:40:45 +0200
> Cc: 20150 <at> debbugs.gnu.org
> 
> I've attached an updated patch without this FIXME.  Other than that,
> it's identical.

Thanks.

> +(define-obsolete-variable-alias 'bookmark-annotation-name
> +  'bookmark-annotation--name "27.1")

Hmm... we declare a non-internal variable obsolete and replace it with
an internal variable?  Why is that a good idea?  Does the new variable
have to be internal?

Message #34 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sat, 13 Jul 2019 15:09:39 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> > +(define-obsolete-variable-alias 'bookmark-annotation-name
> > +  'bookmark-annotation--name "27.1")
>
> Hmm... we declare a non-internal variable obsolete and replace it with
> an internal variable?  Why is that a good idea?  Does the new variable
> have to be internal?

I thought about it a bit more, and I guess it doesn't have to be internal.  I
suggest that the new variable should be kept internal for now though.

I also don't know why I named the new variable bookmark-annotation--name, so
I've changed that to bookmark--annotation-name to better reflect current
practices.

Please see this updated patch with those two changes.

Thanks,
Stefan Kangas

[0001-Restore-focus-to-Bookmark-List-after-editing-annotat.patch (text/x-patch, attachment)]

Message #37 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sat, 13 Jul 2019 16:24:56 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sat, 13 Jul 2019 15:09:39 +0200
> Cc: 20150 <at> debbugs.gnu.org
> 
> > Hmm... we declare a non-internal variable obsolete and replace it with
> > an internal variable?  Why is that a good idea?  Does the new variable
> > have to be internal?
> 
> I thought about it a bit more, and I guess it doesn't have to be internal.  I
> suggest that the new variable should be kept internal for now though.

You mean, it doesn't have to be obsolete?

> I also don't know why I named the new variable bookmark-annotation--name, so
> I've changed that to bookmark--annotation-name to better reflect current
> practices.

Hmm... I don't see bookmark--annotation-name in the patch.  Did you
send the correct patch?

Message #40 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sat, 13 Jul 2019 15:51:57 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:
> > I also don't know why I named the new variable bookmark-annotation--name, so
> > I've changed that to bookmark--annotation-name to better reflect current
> > practices.
>
> Hmm... I don't see bookmark--annotation-name in the patch.  Did you
> send the correct patch?

Oops, please ignore that part.  I wrote the wrong variable name, and
actually meant to say:

I also don't know why I initially named the new variable
bookmark-annotation--from-bookmark-list, so in the updated patch I've
changed that to bookmark--annotation-from-bookmark-list to better
reflect current practices.  (Note the placement of dashes.)

bookmark-annotation-name is unchanged in the updated patch.  I didn't
rename it nor make it obsolete.  I don't think it matters too much --
it could be internal, but it might not be worth renaming.

Sorry for the confusion.

Thanks,
Stefan Kangas

Message #43 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 20150 <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sat, 13 Jul 2019 21:48:30 -0400

Off topic, but it seems to me like you should request write access so
you can apply changes directly. I believe you use
"request for inclusion form" at
https://savannah.gnu.org/project/memberlist.php?group=emacs

Message #46 received at 20150 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 20150 <at> debbugs.gnu.org, stefan <at> marxist.se
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sun, 14 Jul 2019 09:04:29 +0300

> From: Glenn Morris <rgm <at> gnu.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  20150 <at> debbugs.gnu.org
> Date: Sat, 13 Jul 2019 21:48:30 -0400
> 
> 
> Off topic, but it seems to me like you should request write access

He did.

Message #51 received at 20150-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 20150-done <at> debbugs.gnu.org
Subject: Re: bug#20150: 24.4: bookmarks: return from editing annotations (FIX
 INCLUDED)
Date: Sun, 14 Jul 2019 09:29:01 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sat, 13 Jul 2019 15:09:39 +0200
> Cc: 20150 <at> debbugs.gnu.org
> 
> Please see this updated patch with those two changes.

Thanks, pushed to the master branch.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.