bug#34908: Push mark in xref-push-marker-stack

[Dmitry Gutov <dgutov@yandex.ru>]

On 18.03.2019 23:12, Juri Linkov wrote:
> X-Debbugs-CC: João Távora <joaotavora@gmail.com>, Dmitry Gutov <dgutov@yandex.ru>
> 
> Shouldn't xref-push-marker-stack push the mark like all normal commands do?

It's not a command, though. Right?

> I know there is ‘M-,’ but why not allow using the standard keys
> ‘C-x C-SPC’ (pop-global-mark) and ‘C-u C-SPC’ (in the same file)
> as well?

IMO that separation of marks between local and global ones, and 
navigation between them (where you have to remember whether your 
previous navigation was between files or inside one file) is extremely 
counter-intuitive, so I don't have a lot of experience with that facility.

Even so, I think it's been nice enough that every command can choose 
whether it pushes the mark to the local/global buffer rights, and/or it 
adds it to the xref marker stack. Do we have any particular guidelines 
in the manual for when either should happen?

xref-push-marker-stack is used externally as well as a replacement for 
find-tag-marker-ring (which is now marked obsolete). And any command 
that replaced the usage of the latter with the former, and also intended 
to push mark, probably does as a separate action.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index 6974d00048..861e24a85f 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -365,6 +365,7 @@ xref-set-marker-ring-length
>   
>   (defun xref-push-marker-stack (&optional m)
>     "Add point M (defaults to `point-marker') to the marker stack."
> +  (push-mark nil t)
>     (ring-insert xref--marker-ring (or m (point-marker))))

At the very least, it's missing a docstring update.