|
From: | Drew Adams |
Subject: | bug#27193: 25.2; tmm should use completing-read-default |
Date: | Fri, 2 Jun 2017 14:25:42 -0700 (PDT) |
Fair enough. My post certainly assumes quite a bit familiarity with tmm. I'm happy to elaborate below. I am familiar with tmm. I don't care much about it (or for it). I use LaCarte instead.
tmm's weirdness is not in the actual call to completing-read. That completing-read call is wrapped by "(minibuffer-with-setup-hook #'tmm-add-prompt ...)", which in turn calls tmm-define-keys, which sets up a bunch of non-standard key bindings, which have the effect of implementing a menu system with single-keypress activation of menu items, rather than completion of a substring with string matching. I could have added that to the list of things that I do not find extraordinary or problematic in general for `completing-read-function': the key bindings, the completion function, all of it. The case that needs to be made for your proposal is really (IMO) that there are NO values of `completing-read-function', apart from `completing-read-default', which would be compatible with the tmm code. I don't think you can make that case. I sense you are extrapolating from your own particular use of `completing-read-function' (and similar uses). The result is not even recognizable as completing-read. I don't think so. Certainly no more unrecognizable than Ido, which doesn't use *Completions*, defines its own keys, etc. There is lots of room for different uses of `completing-read', and some of those uses might not look much like the behavior of `completing-read-default'. Nothing wrong with that, in itself. And whether a user might or might not realize that `completing-read' is being used is not relevant, IMO. And your proposed fix is simply to impose `completing-read-default', which hardly supports your argument that tmm provides some kind of anomalous, non-completing-read `completing-read'. `completing-read-default' is the canonical `completing-read' behavior, if there ever was one. And that's the behavior that tmm provides by default. The use of completing-read is merely an implementation detail How so? Please think about providing a different implementation of tmm, which doesn't use `completing-read'. That will let us know whether `completing-read' is used only as an implementation detail. in a system that is *not* actually doing completion of any kind. Of course it does completion. Erase chars in the minibuffer from the right and then hit TAB - completion. It's not a completion to write home about, but it's completion. So pluggable completion backends don't make any sense for tmm, That too sounds overly broad. I think that you have too readily convinced yourself that this and that don't make any possible sense, and you are prone to nail things down to prevent what you see as the nonsensical. If such were truly nonsensical then they would likely never exist, and there would be no reason to forbid them. I sense that you think of "pluggable completion backends", that is, uses of `completing-read-function', as necessarily something similar to what your code does. [FWIW I don't think of `completing-read-function' as providing only a "pluggable completion backend". Do you think of `isearch-search-fun-function' as a "pluggable search backend"? Backend? frontend? There are many angles to something like completion or search or... There is not just a backend and a frontend. Likewise, what you call an "alternative completion system". Really, we're just talking about a completion function - a value for `completing-read-function'. That has wide scope - there are few constraints on what it must do. I don't see the justice in adding a constraint for tmm that it cannot do anything except `completing-read-default'.] and I can't imagine any other value of completing-read-function that would make sense for tmm besides completing-read-default. All you need to imagine is something similar to `completing-read-default' but slightly different in some respect. That's all you need to imagine, but nothing prevents more imagination. And what you or I can imagine is not really the point. The question is whether tmm.el must limit itself to `completing-read-default'. I don't see why it should, just because we know of some values of `completing-read-function' that don't do the right thing for tmm. Looking at the "git blame" output for tmm, that call to completing-read has definitely not been updated since completing-read-function was introduced except for minor bugfixes, That's irrelevant. so it makes sense that tmm would be expecting one and only one implementation of completing-read. That does not follow at all.
I'm not trying to set a general precedent here. tmm is the only code that I'm aware of that uses completing-read in this way. I agree that it is not a common way of using it. It doesn't follow that the only possible value of `completing-read-function' that is compatible with tmm is `completing-read-default'. Not at all.
The alternative completing-read-function providers that I am aware of are of are ido-ubiquitous (written by me), ivy, and helm. I'll also include icicles, even though uses some other mechanism besides modifying completing-read-function. ido-ubiquitous and ivy both have code to deactivate themselves when completing-read is called by tmm because otherwise their completion systems would break it, while icicles and helm simply break tmm when enabled. Thus, to my knowledge there is currently no other completing-read-function that doesn't break tmm (except for those that make exceptions specifically for tmm). Again, it is irrelevant that there are uses of `completing-read-function' that break tmm. And what you or I am aware of, or even what might exist anywhere today, does not define the scope of possibilities. [I drink values of the variable `liquid'. Some values, such as strong sulfuric acid, are quite incompatible with my proper functioning. That doesn't mean that the only possible value or the only compatible value for me is the default value, `water'. I drink blindly - I don't know what's in the glass. The only requirement my mouth imposes is that the variable value be a liquid. It is up to whoever fills my glass to DTRT.] And if those uses of `completing-read-function' are incompatible with tmm, and they thus deactivate themselves for tmm commands, that is exactly the right approach, IMO. It is exactly that which I suggested to you (without knowing that that is what you already do). [Tmm does work with Icicles, BTW. (But Icicles does not use `completing-read-function', among other reasons because it wants to work also with older Emacs versions.)]
Based on my explanation above, that is precisely what I think tmm should do: avoid using completing-read-function entirely. I know you think that, but I don't see why. There are surely values of `completing-read-function' that do not bother tmm. We know of one already: `completing-read-default'. Why would you suppose that there can be no others? To look at it another way, tmm was originally written to use completing-read as an implementation detail, and later the function that used to be called completing-read was renamed to completing-read-default, but tmm was never updated to use the new name. This patch rectifies that. That's completely imagination. `completing-read-default' is not the new name of what was once `completing-read'. `completing-read' has never used code like `completing-read-default'. (It has always been written in C.) What I would say that perhaps you will think goes a bit in your direction is this: If you can rewrite `tmm.el' so that it has the same behavior without using `completing-read', and the code is at least as simple and easy to maintain, then I'd say go ahead and do that. That would support your idea that `completing-read' is only an implementation detail etc., and the question of `completing-read-function' would become moot.
This is exactly how ido-ubiquitous and ivy both currently work: they essentially have a blacklist of callers for which they revert to standard completion. tmm is on the blacklist for both packages. That's great. Problem solved. That's the right approach, IMO. Certainly, for any alternative completion system Any? Again with the superlatives. There is more to the world... there will be cases where it needs to fall back to standard completion. In my view, the completion system should be able to determine purely based on the calling context (i.e. its arguments and any relevant dynamically-bound variables) whether or not it needs to punt. Making this decision based on the name of the caller instead of the context to make this decision is admitting that not enough context was provided. I view it as a workaround, not a desirable design pattern, and someday in the future I hope to be able to remove the blacklist from ido-ubiquitous. In the case of tmm, the best heuristic I can think of would be to inspect the key bindings of all the letters and numbers. If any of them are locally rebound in the minibuffer to something other than self-insert-command, then punt. That would be silly, IMHO. There can be plenty of uses of `completing-read' where letter or number keys are bound to commands other than `self-insert-command'. [FWIW, though not directly relevant, when Icicle mode is on there are no keys bound to `self-insert-command' in any of the minibuffer completion keymaps. (But there are keys bound to `icicle-self-insert'.)] However, this doesn't work in practice because the bindings happen in minibuffer-setup-hook, so putting a check at the front of this hook is too early, and putting it at the end is too late because (in the case of ido-ubiquitous) an error is triggered before reaching the end of the hook. This was partly my motivation for suggesting the change in tmm rather than working around it in ido-ubiquitous: because the blacklist approach is the only way for ido-ubiquitous to fix it. It sounds like it is already doing things the right way. (And yes, it might mean a slight maintenance chore for you if libraries such as tmm change significantly.)
For the reasons described above, I would be very surprised if there was *any* alternative completion system that didn't break tmm. Just pick a value of `completing-read-function' that is only slightly different from `completing-read-default', to convince yourself otherwise. I really think you have your particular use case too much in mind, here. `completing-read-function' just means something that uses the same arguments as `completing-read-default' and does something somewhat similar. Its marching orders are pretty general.
As mentioned above, tmm is the only code I'm aware of that does anything like this, and I don't see this as a general approach to be applied anywhere else. OK, that's reassuring. Hopefully the above clarifies my rationale in proposing this change. Thanks for clarifying. I hope my comments above clarify my point of view also, and I hope they help. |
[Prev in Thread] | Current Thread | [Next in Thread] |