Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: search first character of each word #644

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

christoph-heinrich
Copy link
Contributor

@christoph-heinrich christoph-heinrich commented Sep 23, 2023

I don't really understand why title and hint characters should be concatenated.
It might make some sense when the title is only one word, but otherwise it might lead to unexpected results.

I tried also a different implementation based on find instead of gmatch that I expected to be faster, because it only allocated half as many strings, but it turned out to be the same speed.

Ref. #625 (comment)

@tomasklaen
Copy link
Owner

It matches non-word characters currently. For example for string "Delete file & next" it'll create a keyword df&n instead of dfn.

@tomasklaen
Copy link
Owner

And yeah, title and hint probably shouldn't be concatenated, but matched separately.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 23, 2023

We could go with a simple %w, but that doesn't work with non latin alphabets.
We could do something similar to what we're doing for word wrapping and make a list of characters that separate words.

@christoph-heinrich christoph-heinrich force-pushed the first_char_search branch 2 times, most recently from 8a385fc to 09cf2e8 Compare September 23, 2023 14:53
@christoph-heinrich
Copy link
Contributor Author

I've made a list with word separators, I hope I got everyone's favorite word separation characters in there.

@tomasklaen
Copy link
Owner

tomasklaen commented Sep 23, 2023

Add +&()[]{}.

And I guess put word_separators outside of the function since it's static.

@christoph-heinrich christoph-heinrich force-pushed the first_char_search branch 2 times, most recently from d1e11cb to 49ae91f Compare September 23, 2023 16:10
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 23, 2023

wrap_text also has two static lists, idk if you want to do the same thing there. If so I'll do that in the other PR that already touches wrap_text.

@tomasklaen
Copy link
Owner

Oh, and /\, sorry 😐.

wrap_text also has two static lists, idk if you want to do the same thing there

Yeah that would be nice.

@christoph-heinrich christoph-heinrich force-pushed the first_char_search branch 2 times, most recently from e201373 to 6e9b953 Compare September 23, 2023 19:59
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 23, 2023

I missed so many characters 😞
Now that I'm writing this I realize <> should probably also be on that list.

@tomasklaen tomasklaen merged commit c277aca into tomasklaen:main Sep 23, 2023
@junguler
Copy link

amazing work guys, i really appreciate it

tam1m pushed a commit to tam1m/uosc that referenced this pull request Oct 2, 2023
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.

3 participants