Re: [PATCH] emacs/show: Only recenter interactively

[Pengji Zhang <me@pengjiz.com>]

Damien Cassou <damien@cassou.me> writes:

> [...]
>
> That makes sense. This function is, for example, called by
> `notmuch-show-apply-state' which is itself called by
> `notmuch-show--build-buffer' which is itself called by
> `notmuch-show-refresh-view'. If I go with the idea of adding an
> `adjust` parameter as you (and Emacs maintainers) suggest, does it
> mean that I should add this parameter to all these functions? Or maybe
> should I move the call to `notmuch-show-message-adjust' from
> `notmuch-show-goto-message' to interactive functions that (possibly
> indirectly) make use of it (e.g., `notmuch-show-refresh-view' and
> `notmuch-show-filter-thread'). The second solution seems to make the
> most sense to me because it is only in the interactive function that
> the developer really know what should be shown to the user and why.
>
> Do you have another idea or a preference?

I like the second solution better as well. To me, it generally feels
cleaner if we constrain the user-visible side effects in interactive
commands. However, I am afraid it would be involved, and I am not sure
if it is worth the effort.

Besides, do you have other use cases for calling those commands
non-interactively? If not, IMO this time it might be better to only
change 'notmuch-show-previous-message' (or
'notmuch-show-imenu-prev-index-position-function'), so to fix the issue
you had with 'which-function-mode'. After a closer look, I have an
impression that those commands are meant to be interactive only.

Regards,
Pengji