Split hinter tokens at Unicode word boundaries

[stfacc]

Partial completion behaves now like usual word movements by C-Left and C-Right.

This is also useful to accept only parts of a suggested path.

[codecov]

Codecov Report

Merging #650 (7cd131d) into main (973dbb5) will increase coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #650      +/-   ##
==========================================
+ Coverage   49.17%   49.21%   +0.03%     
==========================================
  Files          42       43       +1     
  Lines        7921     7915       -6     
==========================================
  Hits         3895     3895              
+ Misses       4026     4020       -6     

Files	Coverage Δ
src/hinter/cwd_aware.rs	0.00% <0.00%> (ø)
src/hinter/default.rs	0.00% <0.00%> (ø)
src/hinter/mod.rs	0.00% <0.00%> (ø)

[amtoine]

i'm not familiar with Reedline but for people that might want to review this PR, could you add a bit more information? 😇

1. is this addressing a bug?
2. how to test the fix / the feature?

[stfacc]

Sure, the main issue for me is the following:

• Suppose you typed this command
  > ls /some/very/long/path[ ]
  where [] indicates the cursor. Hitting C-Left and C-Right you can move between all elements of the path, one step at a time, for instance
  > ls /some/[v]ery/long/path
• Now you start typing ls at a new prompt and you get
  > ls[ ]/some/very/long/path
  where everything after the cursor is the completion hint.
  After hitting C-Right once, the entire path is now accepted:
  >ls /some/very/long/path[ ]
  This is different from the above situation and, at least for me, unexpected: what I usually want is to accept only part of the suggested path and modify e.g. the final elements.

This happens because currently the next history token is identified using only whitespace, while movements in src/core_editor/line_buffer.rs take into account Unicode words.

• A similar problem happens with long pipelines without spaces around |, which is common in interactive usage. These two history elements behave differently:
  > some|very|long|pipe
> some | very | long | pipe

As additional context, the proposed changed is the mostly the same as fish history completion.

[stfacc]

A few issues related to this one:

• Incorrect word boundary in VI mode #563
  No difference between "cutwordleft" and "cutbigwordleft" #570
  Here it's pointed out that ., :, _ should probably also be used as word separators.

• clean up control-w in default config and make it behave like bash nushell#10288
  Points out that in fish, separators are included in the token. With current unicode_segmentation implementation each separator is its own word, which is not super useful: in the path example above, the cursor would stop at each /

[amtoine]

thanks @stfacc 🙏 😊

[sholderbach]

Thanks for tackling this. Sounds like a good possible quality of life improvement.

Looks pretty good based on how we currently deal with words in src/core_editor/line_buffer.rs.

I think it would be great if we could reduce the copy pasted code a bit here. I think we can make the is_whitespace_str a crate level helper, and maybe you could figure out a good place to share the word advancing for the hinters.

reedline/src/core_editor/line_buffer.rs

Lines 751 to 754 in 973dbb5

	/// Match any sequence of characters that are considered a word boundary
	fn is_whitespace_str(s: &str) -> bool {
	s.chars().all(char::is_whitespace)
	}

[stfacc]

Hi,
I moved the helper functions to the base crate.

Since this is basically the same logic used for the LineBuffer, would it be useful to refactor even more and share the same code?
Not sure how easy. But could save some duplication if we added more logic to tackle the issues pointed out in the other PRs.

[sholderbach]

Thanks for the refactor! Looks good in testing. We can refine the word boundaries later. (e.g. .. becomes two words, but that is something specific to e.g. Nushell where .. are expected for paths or ranges)