bug#71716: [PATCH] Add new completion-preview-insert-{word, sexp} commands

[Eshel Yaron]

Hi Jules,

Jules Tamagnan <jtamagnan@gmail.com> writes:

> Eshel Yaron <me@eshelyaron.com> writes:
>
>> 2. The temporary buffer where the motion command is executed has a
>>    different major mode than the original buffer, so they might have
>>    different notions of words/sexps.
>
> I was thinking about that when implementing this, even further one could
> have locally changed the value of `find-word-boundary-function-table`
> outside of `subword-mode`.

Right.  And when considering sexps, forward-sexp-function can come into
play, which might take into account all sorts of buffer-local variables.

> One idea I had thought of was inserting the complete after-string and
> performing character deletions until the suffix was removed but this
> felt like an even worse solution.

I think that might be the way to go, actually.  Placing the after-string
insertion and subsequent deletion in an atomic change group (and using
undo-amalgamate-change-group to let the user undo everything in one go)
should hopefully work just as well, and that would alleviate the need to
chase down and replicate complex buffer state in the temporary buffer.

Jules Tamagnan <jtamagnan@gmail.com> writes:

> I've further tweaked the code to address the second point of feedback.

Thanks!

> Looking at it now it seems a bit uglier for the "standard" insert case
> so I'd be willing to revert that consolidation.

I think that'd be best, yes.  Let's keep completion-preview-insert
intact for the time being and see if we there's room for cleanly
consolidating it with the new commands after we get them right.

> Overall it seems to work well both in unit tests and in my personal
> testing.
>
> In the last message I attached a patch with only my second commit. This
> new patch contains of all 3 commits:

I'll give it a try, thanks.  In the future if you could squash all
changes to a single patch I think that'd make it easiest to review.


Best,

Eshel