Hi Spencer, On 16/09/2024 22:54, Spencer Baugh wrote: > Eli Zaretskii writes: >> I'm okay with adding a new style, per B, but why do we need to >> deprecate emacs22 at the same time? Let users who want this new >> behavior customize their completion styles to use this new style >> instead of emacs22. That'd be fully backward compatible, and will >> solve the problem for those who don't like the current behavior. > > That's fair enough, we don't need to deprecate emacs22 at the same time, > we can wait until the new style has been battle-tested. > > I think a new style should replace emacs22 in the default > completion-styles eventually, but that can wait. > > And after working on this bug for a while, I now am convinced that the > new style approach is straightforward, and is the best way. (Although > it would also work to make these changes in the emacs22 style) I'm not quite convinced about the "new style only" approach myself, but anyway we can discuss a solution that could be applied to any style, opt-in or opt-out. > Attached is a patch which adds a new ignore-after-point style. The fix > for this bug is entirely contained in the differences between > completion-ignore-after-point-all-completions (c-iap-a-c) and > completion-emacs22-all-completions (c-e22-a-c). > > c-e22-a-c always omits the text after point, which means > choose-completion always deletes that text, causing this bug. > > c-iap-a-c includes the text after point in some cases. Whenever the > text after point is included, it's grayed out with a new > completions-ignored face to indicate it was ignored. > > The cases where c-iap-a-c includes the text after point are those where > the user will have further chances to edit or complete with that text: > > - When we're doing minibuffer completion and choose-completion will > immediately exit the minibuffer, the text after point is not included, > since the user won't get any further changes to use it, and it's > probably garbage. > > - When we're doing completion-at-point (or certain kinds of minibuffer > completion, e.g. completing a directory in filename completion), the > text after point is included. In such cases, the user can always > delete it themselves later, or might want to actually use it. > > To make this work consistently with completion-try-completion (which > keeps point before the ignored suffix in both the ignore-after-point and > emacs22 styles), choose-completion-string now checks a new > 'completion-position-after-insert text property, and moves point to that > position. > > (There are two other changes which are general improvements unrelated to > this bug: > > - The behavior of keeping point before the ignored suffix is more > consistent. > > - Instead of hardcoding try-completion and all-completion, > ignore-after-point uses the configured completion styles.) Thank you, I see a few problems with that approach (as discussed privately as well). To recap: From my POV integrating the result with company-mode is non-trivial. And the created visual doesn't seem optimal in a number of edge cases (long prefix = weird-looking popup; this probably applies to the *Completions* buffer as well, though maybe to a lesser extent). Another problem is both the "all-completions" method and the insertion function call out to UI: choose-completion-string--should-exit references minibuffer-completion-table and completion-no-auto-exit directly, for example. I'm on the fence about coupling to the completion category. Finally, the use of 'completion-position-after-insert' seems like it could be used separately from the "ignored text", meaning the spans don't have to match. So it could be a separate feature, one that's easy enough to implement on its own. None of the above would be insurmountable, but here's what I think avoids it using the 'completion--adjust-metadata' thingy that was originally added for 'flex' a few releases ago: adding a metadata key 'completion-ignore-after-point'. The attached patch does not make a distinction for file name completion - it just covers the core problem - but I think any UI could use the addition and likewise lookup the 'file' category, and print the "hidden" suffix in the Completions, and decide to drop the suffix in your first scenario (file name completion with exit imminent). Spencer, Stefan, WDYT? Again, the patch is against the emacs22 style for readability, but a new style can be added just as well.