Subject: Re: bug#33870: 27.0.50; xref-goto-xref not configurable

From: João Távora <[email-masked]>
Date: Tue, 08 Jan 2019 01:04:49 +0000

Juri Linkov <[email-masked]> writes:

> Of course, it doesn't work if you tried it only with part of my changes.
> When I submitted my initial patch, I tested it in all your test cases,
> including the above test case that was not broken with my patch.

You are correct.  I was testing under the assumption that making
xref-goto-xref configurable didn't require the "tiny window" for
xref-find-definitions, which is something you stated that you wanted to
do for other xref.el commands like xref-find-references.

Anyway, can't you add the tiny window xref-find-definitions and the
other UI changes as an addon to *my* patch?

> But you asked to break my patch to several pieces and submit them
> separately to different bug reports.  No wonder that each of them
> doesn't do what the whole patch did.
>> Expected xref.el to appear in the bottom window which was my original
>> intent when I said "other window".
> Then the xref buffer is obscured by another buffer visited in the same
> window, and if the user wants to visit more hits from the xref buffer,
> this is not easy to do.

>> In the current master this works OK, in your patch it doesn't.
>
> My initial patch solved this problem gracefully by creating a new window
> for the xref buffer.

You may well call this a problem, but it's not a bug.  It's the defined
behaviour, it's like this by design.  We are trying to create the
conditions that would enable you, or any other user, to create
alternative ways to present *xref* that have other advantages and
drawbacks.

>> I've also renamed window.el's window--display-buffer to
>> window-display-buffer throughout Emacs (i.e. made it public).
> You can't rename old functions lightly.  This will break the existing
> code.

What other code?  I renamed the uses in Emacs too: do you mean code
outside Emacs?  It shouldn't be using that internal function in the
first place.  But if it is, no reason to punish it: we can just provide
a deprecated function alias, which will at most warn its developers of
the imprudency.

No.  I can because they were internal functions that no-one was supposed
to have been using in the first place.  *That* is precisely why internal
functions are useful, so you can decide when and if to make them public.

You may argue that by making them public *now* we are going to have a
more deprecation problem if we decide to rename them again *in the
future*.  I would agree with you there.

>> After we merge this, we can continue the discussion about the changing
>> the xref UI in the other bug you opened, bug#33992

> Better start with bug#33992 because it supports the above test case,
> then we could finish this bug#33870.

No.  There is consensus on fixing the bug (xref.el doesn't respect
display-buffer-alist et al) without changing the UI.  There is no
consensus on changing the UI.

Also I think some would not really view this is a bug at all.

It would seem Juri that you are trying to shoehorn a behaviour change
into a bugfix.  Both things have less chances to go into Emacs that way.
I'm sure that:

* you can implement the UI changes that you want on top of my patch;

* you can make them optional;

* we can then agree on a good default.

Is that not so?

João