[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: completion-ui.el -- new release

From: Toby Cubitt
Subject: Re: completion-ui.el -- new release
Date: Mon, 23 Apr 2007 17:29:01 +0100
User-agent: Thunderbird (Windows/20070221)

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.

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]