Re: [PATCH 1/3] git-blame.el: Do not use goto-line in lisp code

From: Lawrence Mitchell <wence@gmx.li>
Date: Jun 14, 2012, 09:14 UTC
Message-ID: <87k3za9rwj.fsf@gmx.li>
In-Reply-To: <20120614050854.GG27586@burratino>

Jonathan Nieder wrote:

> Hi Lawrence,

> Lawrence Mitchell wrote:

>> From: Rüdiger Sonderfeld <ruediger@c-plusplus.de>

>> goto-line is a user-level command, instead use the lisp-level
>> construct recommended in Emacs documentation.
> [...]
>> Here we go, all Rüdiger's changes look sensible, I've split them into bits though

> Thanks for looking them over.

> Would you mind indulging my curiosity a little by describing what bad
> behavior or potential bad behavior this change prevents?

goto-line sets the mark, and respects the variable selective-display. It also widens the buffer before moving to the relevant line. The first two are almost never what you'd want in lisp code, the latter you'd probably want to make explicit in the calls I guess.

the with-current-buffer issue is a bit more subtle, and I realise my patch for this didn't actually fix the bug, or describe the problem properly (reroll to come).

Basically:

save-excursion saves point, mark and current-buffer in the buffer in scope when it is called, but if we do:

(save-excursion
  (set-buffer buf)
  ;; modify point and mark in buf
  ...)

hoping to save point and mark in buf, it doesn't happen. Instead, we need to make buf current before calling save-excursion. And we want to restore the value of current-buffer in scope at the beginning of the call afterward, hence the correct idiom is:

(with-current-buffer buf
  (save-excursion ...))

Cheers,

Lawrence

-- 
Lawrence Mitchell <wence@gmx.li>