rain-1 / linenoise-mob

A small self-contained alternative to readline and libedit
BSD 2-Clause "Simplified" License
66 stars 10 forks source link

[WIP] Add support for completing words instead of lines #38

Closed nmeum closed 5 years ago

nmeum commented 5 years ago

I personally need to complete words, not entire lines and quickly hacked something together in order to figure out how easy it is to implement this in linenoise. This isn't finished yet but it works okish and I just wanted to hear your thoughts on this. The code is a bit ugly since string handling with C is somewhat tricky and the changes include an API change.

Demo:

asciicast

I tried to make the changes as less intrusive as possible. This was achieved by only passing the current word to the completion callback and expanding all returned completions afterwards. This obviously isn't the most effective way to implement this (from a memory usage perspective) but it required only few changes to existing functions.

Limitations: Doesn't work with completions containing spaces.

TODO:

rain-1 commented 5 years ago

This does look good and seems like something that could be of use to other users. Keep it up!

nmeum commented 5 years ago

Removed the dependency on memrchr(3). Regarding the other TODOs: Proper support for completions containing spaces and for completions which are shorter than the existing word can only be added if the end position of the expanded completion is stored somewhere. For example in the linenoiseCompletions type or as a return value of callCompletionCallback. Otherwise updateCursorPos will not work correctly in these cases.

An alternative solution would be always setting the cursor to the end of the line even when completing words which is easier to implement but a bit less intuitive imho.

What are your thoughts on this?

rain-1 commented 5 years ago

I think it makes the best sense to add it in the return value of callCompletionCallback, it would not be the right UI if the cursor jumps to the end after any completion.

nmeum commented 5 years ago

It seems to me that proper support for placing the cursor after the expanded word will be difficult without deeper changes to the existing codebase.

If we want to handle this in completeLine (as you seem to prefer) we need to expand completions in the while (!stop) loop but in order to that we need to store which strings in lc->cvec have already been expanded otherwise they are expanded again.

If we want to expand completions in callCompletionCallback (as implemented currently) we need to store the end position of the expanded completion/word somewhere.

So in both cases additional metadata would be required.

rain-1 commented 5 years ago

OK. Thanks for doing that analysis. I see what you mean. I think there is a fair bit of tricky work with array indices and stuff, and there is also the issue of testing all this. It's good that you identified the < length case. Maybe thinking about the problem in abstract will help to produce the cleanest design:

For completion you basically have a line of text and a cursor position in there: buffer, index. At the end of completion both items have been modified.

word completion can be thought of as a special type of completion: It takes the current word, looking up for completions of that, then inserts the rest of that word into the buffer and moves the cursor on.

Another possible completion system could be this one, useful for shells https://deftly.net/posts/2017-05-01-openbsd-ksh-tab-complete.html On the OSH blog there is some discussion about completion too http://www.oilshell.org/blog/2018/12/05.html

nmeum commented 5 years ago

Just to clarify: The code currently works as is. The only remaining issues is updating the cursor position correctly. Since (as I explained above) this is not easily doable I decided to implement completions for the last word in my linenoise-based program (the caller) instead of changing the linenoise completion API.