Re: completion-ui.el -- new release

[Toby Cubitt]

Maciej Katafiasz wrote:
> Den Wed, 14 Feb 2007 20:08:18 +0000 skrev Toby Cubitt:
>
> > ;;; completion-ui.el --- in-buffer completion user interface
> >
> > ;; This package provides a user-interface for in-buffer text
> > ;; completion. It doesn't find completions itself. Instead, a completion
> > ;; package can simply set the `completion-function' variable to a function
> > ;; that takes two arguments, a string PREFIX and an integer MAXNUM, and
> > ;; returns a list of at most MAXNUM completion candidates for
> > ;; PREFIX. Completion-UI does the rest.
>
> I can whole-heartedly agree with the above idea,

Good :) At least we share a common goal...


> but I can see some
> problems with the implementation. First, the package is fairly confusing
> from the user's perspective, especially with all methods being enabled by
> default and no clear decision on which one will actually get used. There
> should be a single customisation to say that, instead of each method
> having an on/off toggle. They are mutually exclusive after all.

No, they are not. It makes perfect sense to use a number of them 
simultaneously. In fact, that is exactly what I do. Forcing a user to 
select one and only one would drastically reduce their freedom of choice.

I agree that enabling all(*) methods by default is not perfect, but it 
is at least the most logical choice. Anything else would simply reflect 
my own personal preferences. Enabling all of them allows the user to see 
what's available. After all, it's very easy to disable them.

(*) In fact, the most intrusive methods, such as 
completion-auto-show-menu, are disabled by default.


> Second, I've had very bad luck trying to use it, my quick hack of a
> function[0] revealed a couple of bugs: 

No. It revealed that if you deliberately ignore the documentation and 
the function interface, and use the function in a way that was never 
intended, it doesn't work. Not exactly rocket science.


> 1) it doesn't actually work :)

It works perfectly well if you use it correctly. Please don't confuse 
"bug" with "feature request". You raise some interesting points, but 
none as far as I can tell are bugs.


> when trying to commit it doesn't dismiss
> the popup and the result is a garbled mess with a completion candidate
> still haging around. The non-workingness might be related to:
>
> 2) it doesn't have any concept of separation between what's presented as
> the completion, and what's actually inserted when the user commits. Why it
> is useful to distinguish is immediately demonstrated by the following
> ASCII-artish completion popup:
>
> ba __________________
>   |int     bar       |
>   |float   baz       |
>   |FooBar *ballooney | => `ballooney' gets inserted
>   `------------------'

This is a good point, and I will consider supporting something like this 
in a future version of completion-UI.


> Incidentally, this is exactly what my completion function returns, for
> reasons that will become apparent in a moment. And that breaks
> completion-ui.el horribly.

Again, it's hardly surprising that if you deliberately supply arguments 
to the completion-UI functions that are not in the required form, it 
breaks. This can more accurately be described as a bug in your code than 
in mine (or, if I'm being more generous, it is a feature request phrased 
in a rather unfortunate way).


> 3) I had even worse of luck trying to get the tooltip display method to
> work than with dynamic completion. For some reason as soon as the tooltip
> is presented, the garbled mess mentioned above is inserted at point, so now
> I have to aggressively orange garbled messes presented (one of them
> being the tooltip) and no clear way of getting out ;)

I don't follow your description. Could you please provide a step-by-step 
explanation of how to reproduce this? And please mail this to me 
off-list; this is not the appropriate forum for this kind of discussion. 
However, if the problem only occurs with your faulty 
completion-function, then it is not a problem with completion-UI but 
with your function. Completion-UI works fine for all the completion 
functions I have ever written, or helped others to write.


> 4) I don't like the way you do your keymaps at all, and I happen to know
> you can do _much_ better than that, again for reasons that will become
> apparent in a moment. Creating maps based on some hardcoded set of
> "printable" characters gets a huge :( from me.

I suggest you read the code properly before making wildly inaccurate 
comments. The hard-coded set of characters is only used as a work-around 
in older versions of Emacs. In any version that supports command 
remapping (both stable and CVS), it instead uses that much more elegant 
method.


[snip]
> It does the above by the magic of default keybindings and judicious
> lookups / replays of events.

This is dangerous territory, and runs the risk of misbehaving badly with 
certain combinations of packages or minor-modes. At the very least, 
trying to ensure yours is the last keymap to be loaded is not robust. 
Completion-UI stays out of the way of other packages by only using 
minor-mode and overlay keymaps, which (being standard Emacs features) 
are much less likely to cause trouble.

(I should mention for completeness that it uses an ugly work-around 
similar to the one you describe for older Emacs versions that don't 
support the relevant features. Since newer Emacs versions provide a much 
more elegant solution for this kind of thing, there's little incentive 
to make the word-around elegant, even if it were possible.)


> Now I would be very glad to have the tooltip-related code split out and
> reusable by anyone, and I wanted to add another display method anyway,
> namely buffer-inline pseudo-tooltip rendered with character overlays,
> which I intend to steal from uim.el. But in order for completion-ui.el
> being that common display library, it needs to be improved considerably.

So far you've made a number of inaccurate comments and repeatedly said 
that the completion-UI code is badly written, without describing 
anything that's actually wrong with it. In fact, although I can think of 
many additional features that would be very desirable in completion-UI, 
the code is (I think) reasonably tidy. It is, for example, very easy to 
add a new completion method:

1) Write a new function to implement it, which takes the completion 
overlay as an argument (from which the prefix and list of completions 
can be obtained as properties).

2) Define one new customization variable to enable/disable the new 
completion method.

3) Add one more line to the complete-in-buffer function, using the 
existing ones as a template and making the obvious modifications.


What exactly is it that is so bad or that is missing from completion-UI? 
I'm very open to suggestions and comments, but not to inaccurate, 
unconstructive criticism.


> I'd like you to take a look at semantic-hover-completion.el, and steal
> the code when I'm done, as I believe it does a better job of being good
> UI than completion-ui.el rigth now.

If you want to make a useful contribution, first understand properly 
what you're talking about. I would happily have answered questions about 
how and why completion-UI does things (preferably off-list to avoid 
annoying others), which would have avoided the misunderstandings you 
seem to be suffering from. Once you do understand the package, explain 
what the problem is: step-by-step instructions to reproduce the bugs you 
see, or a coherent description of precisely what feature you would like 
added. Or better still, send me a patch to test.

Do not simply tell me your package is better without explaining why, and 
tell me to "steal [your] code when [you're] done". You seem to have no 
desire to read and understand my code before commenting on it. Why on 
Earth do you suppose I would then have any desire to invest time reading 
and understanding yours?

Let's try to take this discussion to a more constructive level, and see 
if together we can improve completion-UI by incorporating your code 
and/or ideas.

Toby