Skip to content

Conversation

@MaxGyver83
Copy link
Contributor

I have created a pull request to rakitzis/rc to integrate bestline: Support bestline for line editing #100.

Unfortunately, bestline isn't ideal to be used in rc as it is now:

To be useful in a shell, it's necessary to pass the cursor position when calling the complete function because the user might want to complete any word (token) in the current command line. I'm aware that this is a breaking change. But it might make bestline more suitable for other projects, too.

This pull request also adds a new libbestline.so target to the Makefile because the rc maintainers prefer linking against a shared library instead of copying the current state of a line editing library (at least they did in the past: rakitzis/rc#5 (comment)).

Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the latency getting back to you. It's really exciting that you're using bestline in rc. This requires an API change. It should be minimally disruptive though. I personally don't mind adding an additional parameter to my completion callbacks. So if your project needs this, then I'm happy to approve. Could you sync this PR to head so I can click merge? Thanks!

@MaxGyver83
Copy link
Contributor Author

MaxGyver83 commented Oct 20, 2024

Thanks for your review! I have rebased this branch and resolved all conflicts (only whitespace changes). Update: I have squashed the first and fourth commit into one.

The cursor position (pos) is important to know if you want to use
bestline in a shell.

Example:
When the cursor is at the end of the first word, you need command
completions. When the cursor is that the end of a word beginning with
`$`, you need variable completions.
@jart jart merged commit 1755e79 into jart:master Oct 22, 2024
@jart
Copy link
Owner

jart commented Oct 22, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants