Don't slice line in DefaultCompleter

[ysthakur]

Goes towards fixing nushell/nushell#8091. nushell/nushell#11488 (which should finish the job) relies on this PR to make only_buffer_difference: true work with the columnar and list menus.

I don't know what exactly the old behavior was supposed to be (I looked at the history of the relevant files but couldn't find any recent commits that seemed to change behavior), so maybe this PR has different behavior for the user (it also changes the contract of Completer::complete). But I think I've narrowed down the issue, so someone else can at least make a new PR based on that.

It seems like the problem was in DefaultCompleter::complete, where it does line[0..pos]:

reedline/src/completion/default.rs

Lines 71 to 75 in f396223

	fn complete(&mut self, line: &str, pos: usize) -> Vec<Suggestion> {
	let mut span_line_whitespaces = 0;
	let mut completions = vec![];
	if !line.is_empty() {
	let mut split = line[0..pos].split(' ').rev();

DefaultCompleter::complete assumes that the line given to it is the whole line and that pos is the cursor position. It also assumes that the content to complete begins at the start of the line and ends at pos. However, with only_buffer_difference set to true, the line passed to the completer is only the changed portion of the buffer, and the argument to pos is the start of the changed portion of the buffer rather than the cursor position. So when you type cd <TAB>c, line is "c" and pos is 3, so line[0..pos] fails.

Although there's a check for the line being empty, there isn't one for pos being out of bounds. I figured it'd be easier to have the menus slice the line before sending it over to be completed so they can decide which portion of the text to complete. pos would then be used solely to determine the spans of the returned Suggestions.

One problem is that although the DefaultCompleter assumes that the argument to pos is the end of the content to complete, as far as I can tell, HistoryCompleter seems to assume that pos is the start of the content's span (edit: I've now changed it to use the end instead of the start, for consistency):

reedline/src/completion/history.rs

Lines 53 to 57 in f396223

	fn create_suggestion(&self, line: &str, pos: usize, value: &str) -> Suggestion {
	let span = Span {
	start: pos,
	end: pos + line.len(),
	};

On the one hand, making pos the start is nice because you add the input's length to get the end. On the other hand, making pos the end is nice because that's the cursor position (unless you have only_buffer_difference set to true). Either way, the doc comment on Completer::complete doesn't seem detailed enough (edit: I've now changed it to say that pos must be the end of the text):

reedline/src/completion/base.rs

Lines 29 to 31 in f396223

	/// the action that will take the line and position and convert it to a vector of completions, which include the
	/// span to replace and the contents of that replacement
	fn complete(&mut self, line: &str, pos: usize) -> Vec<Suggestion>;

(these permalink snippet things are so cool)

I guess it doesn't really matter because the HistoryCompleter isn't used the same way as the DefaultCompleter, but it'd still be nice to have consistency.

Here's a recording of the changes using the columnar menu and only_buffer_difference set to true:

and with the list menu:

There was also a small bug in string_difference where, if the old buffer was of length n and the new buffer was longer than that (say n+1), old_buffer[0..n] == new_buffer[0..n], and old_buffer[n] == new_buffer[n+1], then no difference would be detected. That seems to be fixed now (added one test).

[codecov]

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (f396223) 49.26% compared to head (8eefe0d) 56.09%.
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #695      +/-   ##
==========================================
+ Coverage   49.26%   56.09%   +6.83%     
==========================================
  Files          46       46              
  Lines        7911     7993      +82     
==========================================
+ Hits         3897     4484     +587     
+ Misses       4014     3509     -505     

Files	Coverage Δ
src/completion/base.rs	29.16% <ø> (ø)
src/completion/default.rs	67.85% <100.00%> (+0.49%)	⬆️
src/menu/menu_functions.rs	97.46% <100.00%> (+1.60%)	⬆️
src/engine.rs	22.35% <0.00%> (+17.36%)	⬆️
src/completion/history.rs	0.00% <0.00%> (ø)
src/menu/columnar_menu.rs	33.21% <66.66%> (-0.56%)	⬇️
src/menu/list_menu.rs	8.74% <0.00%> (-0.22%)	⬇️

... and 18 files with indirect coverage changes