Re: [elpa] externals/hyperbole c501027 2/2: Fix set-buffer byte-compiler warnings; remove outdated references

[Robert Weiner]

Eli and I exchanged messages about this on the emacs-devel list and settled on the outer save-excursion as you first wrote below since it is indeed the original buffer whose point we want to save which may be manipulated by some subroutine deep within the with-current-buffer that changes the buffer back to the original.  Thanks as always for the pointers though.  -- Bob

On Mon, May 10, 2021 at 10:00 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>       (save-excursion
> -       (set-buffer (if (bufferp loc) loc (find-file-noselect loc)))
> -       (when (ibut:to (ibut:key but-sym))
> -         (let (buffer-read-only)
> -           (if (< (point) start)
> -               ;; Find beginning of button named label delimiter and delete
> -               ;; from there.
> -               (progn (goto-char (- (point) (length ibut:label-start)))
> -                      (delete-region (point) end))
> -             ;; No label, just delete delimited ibutton text.
> -             (delete-region start end))
> -           (when (looking-at "[ \t]*\r?\n")
> -             (delete-region (point) (match-end 0)))
> -           (run-hooks 'ibut-delete-hook))))
> +       (with-current-buffer (if (bufferp loc) loc (find-file-noselect loc))
> +         (when (ibut:to (ibut:key but-sym))
> +           (let (buffer-read-only)
> +             (if (< (point) start)
> +                 ;; Find beginning of button named label delimiter and delete
> +                 ;; from there.
> +                 (progn (goto-char (- (point) (length ibut:label-start)))
> +                        (delete-region (point) end))
> +               ;; No label, just delete delimited ibutton text.
> +               (delete-region start end))
> +             (when (looking-at "[ \t]*\r?\n")
> +               (delete-region (point) (match-end 0)))
> +             (run-hooks 'ibut-delete-hook)))))
>       but-sym))))
> 
>  (defun    ibut:get (&optional lbl-key buffer key-src)
> diff --git a/hui-window.el b/hui-window.el
> index 654c035..796770e 100644
> --- a/hui-window.el
> +++ b/hui-window.el
> @@ -311,12 +311,12 @@ part of InfoDock and not a part of Hyperbole)."
>    "Return t iff there is a non-empty active region in buffer of the last Smart Mouse Key release."
>    (when (setq hkey-value (if assist-flag assist-key-depress-prev-point action-key-depress-prev-point))
>      (save-excursion
> -      (set-buffer (marker-buffer hkey-value))
> -      ;; Store and goto any prior value of point from the region
> -      ;; prior to the Smart Key depress, so we can return to it later.
> -      (and (goto-char hkey-value)
> -        (hmouse-save-region)
> -        t))))
> +      (with-current-buffer (marker-buffer hkey-value)
> +     ;; Store and goto any prior value of point from the region
> +     ;; prior to the Smart Key depress, so we can return to it later.
> +     (and (goto-char hkey-value)
> +          (hmouse-save-region)
> +          t)))))

These two don't make much sense: `save-excursion` only saves the
position of point in the current buffer, so

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

is useless in one of two ways:
- FOO is already the current buffer, so `with-current-buffer`
  (previously `set-buffer`) does nothing.
- FOO is a different buffer, so the buffer-position saved&restored by
  `save-excursion` is in a buffer that's not affected by `...`.

If you want to combine the two, then you should use

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

Tho if we presume that the `set-buffer` in the previous code did do
something useful (i.e. selected a different buffer), then there's a good
chance that the `save-excursion` has never done its intended job and can
be removed.


        Stefan