bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer

[Geoff Gole]

In ibuffer, running ibuffer-toggle-sorting-mode will produce a useful
message telling the user what the buffer is now sorted by. That
message is immediately clobbered by an useless `Mark set' message
(useless because ibuffer stomps all over any such mark).

To reproduce:

  emacs -Q
  M-x ibuffer
  ,
  C-h e and observe that both "Sorting by ..." and "Mark set" appear

A patch follows (against GNU Emacs 23.0.60.1 (i486-pc-linux-gnu, GTK+
Version 2.12.11) of 2008-11-22 on elegiac, modified by Debian).

diff -c -L /usr/share/emacs/23.0.60/lisp/ibuffer.el.gz -L
/tmp/buffer-content-31328m2 /tmp/jka-com3132uwF
/tmp/buffer-content-31328m2
*** /usr/share/emacs/23.0.60/lisp/ibuffer.el.gz
--- /tmp/buffer-content-31328m2
***************
*** 2277,2283 ****
        (setq buffer-read-only t)
        (set-buffer-modified-p ibuffer-did-modification)
        (setq ibuffer-did-modification nil)
!       (goto-line (1+ orig)))))

  (defun ibuffer-quit ()
    "Quit this `ibuffer' session.
--- 2277,2284 ----
        (setq buffer-read-only t)
        (set-buffer-modified-p ibuffer-did-modification)
        (setq ibuffer-did-modification nil)
!       (goto-char 1)
!       (forward-line orig))))

  (defun ibuffer-quit ()
    "Quit this `ibuffer' session.

bug#2439: marked as done (23.0.60; [patch] `Mark set' clobbers useful message in ibuffer)

[Emacs bug Tracking System]

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


Your message dated Mon, 23 Feb 2009 12:01:57 +0100
with message-id <f7ccd24b0902230301l4ac70462w24994cb492180b20@mail.gmail.com>
and subject line Re: bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in  ibuffer
has caused the Emacs bug report #2439,
regarding 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
2439: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=2439
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 4121 bytes --]

From: Geoff Gole <geoffgole@gmail.com>
To: emacs-pretest-bug@gnu.org
Subject: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer
Date: Mon, 23 Feb 2009 11:26:49 +0900
Message-ID: <f5bc73230902221826h5f405509x9c2ee08b9321e2a7@mail.gmail.com>

In ibuffer, running ibuffer-toggle-sorting-mode will produce a useful
message telling the user what the buffer is now sorted by. That
message is immediately clobbered by an useless `Mark set' message
(useless because ibuffer stomps all over any such mark).

To reproduce:

  emacs -Q
  M-x ibuffer
  ,
  C-h e and observe that both "Sorting by ..." and "Mark set" appear

A patch follows (against GNU Emacs 23.0.60.1 (i486-pc-linux-gnu, GTK+
Version 2.12.11) of 2008-11-22 on elegiac, modified by Debian).

diff -c -L /usr/share/emacs/23.0.60/lisp/ibuffer.el.gz -L
/tmp/buffer-content-31328m2 /tmp/jka-com3132uwF
/tmp/buffer-content-31328m2
*** /usr/share/emacs/23.0.60/lisp/ibuffer.el.gz
--- /tmp/buffer-content-31328m2
***************
*** 2277,2283 ****
        (setq buffer-read-only t)
        (set-buffer-modified-p ibuffer-did-modification)
        (setq ibuffer-did-modification nil)
!       (goto-line (1+ orig)))))

  (defun ibuffer-quit ()
    "Quit this `ibuffer' session.
--- 2277,2284 ----
        (setq buffer-read-only t)
        (set-buffer-modified-p ibuffer-did-modification)
        (setq ibuffer-did-modification nil)
!       (goto-char 1)
!       (forward-line orig))))

  (defun ibuffer-quit ()
    "Quit this `ibuffer' session.



[-- Attachment #3: Type: message/rfc822, Size: 2677 bytes --]

From: Juanma Barranquero <lekktu@gmail.com>
To: Geoff Gole <geoffgole@gmail.com>
Cc: 2439-done@emacsbugs.donarmstrong.com
Subject: Re: bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in  ibuffer
Date: Mon, 23 Feb 2009 12:01:57 +0100
Message-ID: <f7ccd24b0902230301l4ac70462w24994cb492180b20@mail.gmail.com>

On Mon, Feb 23, 2009 at 03:26, Geoff Gole <geoffgole@gmail.com> wrote:

> A patch follows (against GNU Emacs 23.0.60.1 (i486-pc-linux-gnu, GTK+
> Version 2.12.11) of 2008-11-22 on elegiac, modified by Debian).

Installed, thanks.

    Juanma

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer

[Stefan Monnier]

> !       (goto-char 1)
> !       (forward-line orig))))

Thanks for your patch, but please try to avoid hardcoding "1" as above.
Just use (point-min) instead.


        Stefan

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer

[Juanma Barranquero]

On Wed, Feb 25, 2009 at 17:30, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Thanks for your patch, but please try to avoid hardcoding "1" as above.
> Just use (point-min) instead.

What is the point, when having a narrowed buffer would be an error?

In this case,

  (goto-char 1)
  (forward-line orig)

is not the same as

  (goto-line (1+ orig))

because `goto-line' uses `(save-restriction (widen) ...)'. So, if
anything, is not that the patch should use `point-min', but that it
should use

  (save-restriction
    (widen)
    (goto-char 1)
    (forward-line orig))

though I don't think narrowing the ibuffer buffer and then sorting is
a common enough operation to worry about it. Same for bs-down, where I
implemented the same fix.

    Juanma

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in

[Stefan Monnier]

>> Thanks for your patch, but please try to avoid hardcoding "1" as above.
>> Just use (point-min) instead.
> What is the point, when having a narrowed buffer would be an error?

Because 1 is sometimes an error, whereas (point-min) isn't.

> because `goto-line' uses `(save-restriction (widen) ...)'. So, if
> anything, is not that the patch should use `point-min', but that it
> should use

>   (save-restriction
>     (widen)
>     (goto-char 1)
>     (forward-line orig))

No, it should be

   (save-restriction
     (widen)
     (goto-char (point-min))
     (forward-line orig))

> though I don't think narrowing the ibuffer buffer and then sorting is
> a common enough operation to worry about it.  Same for bs-down, where
> I implemented the same fix.

Agreed.  But still using (point-min) rather than 1.


        Stefan

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in

[Juanma Barranquero]

On Wed, Feb 25, 2009 at 20:53, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Agreed.  But still using (point-min) rather than 1.

OK, fixed in these two cases. There are still 31 instances of
(goto-char 1) in 24 files:

autoinsert.el
indent.el
simple.el
emacs-lisp\gulp.el
emacs-lisp\testcover.el
international\ja-dic-cnv.el
international\quail.el
mail\mail-utils.el
mail\mailalias.el
net\ange-ftp.el
nxml\rng-cmpct.el
play\dissociate.el
progmodes\ada-xref.el
progmodes\cperl-mode.el
progmodes\ebrowse.el
progmodes\idlwave.el
progmodes\ps-mode.el
textmodes\page.el
textmodes\reftex-cite.el
textmodes\reftex-dcr.el
textmodes\reftex-index.el
textmodes\reftex-parse.el
textmodes\reftex-ref.el
textmodes\reftex.el

    Juanma

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in

[Stefan Monnier]

>> Agreed.  But still using (point-min) rather than 1.

> OK, fixed in these two cases. There are still 31 instances of
> (goto-char 1) in 24 files:

There's no hurry to change that code.  Just keep it in mind when you
write new code or modify existing code.


        Stefan

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in ibuffer

[Geoff Gole]

> Thanks for your patch, but please try to avoid hardcoding "1" as above.
> Just use (point-min) instead.

Good point. I took a peek at the source of goto-line to see if it did
anything that I'd need to replicate, and grabbed the line from there
without thinking.

Should goto-line also use (point-min)?

> I don't think narrowing the ibuffer buffer and then sorting is
> a common enough operation to worry about it.

I didn't bother with handling restrictions, as they don't seem to be
respected by ibuffer. Narrowed ibuffer buffers will either behave
strangely or simply clobber the restriction.

Perhaps I should have mentioned that in a comment or when submitting the patch.

bug#2439: 23.0.60; [patch] `Mark set' clobbers useful message in

[Stefan Monnier]

> I didn't bother with handling restrictions, as they don't seem to be
> respected by ibuffer. Narrowed ibuffer buffers will either behave
> strangely or simply clobber the restriction.

> Perhaps I should have mentioned that in a comment or when submitting
> the patch.

This issue is unrelated to your patch, really.


        Stefan