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

Bugfix: smart-case does not work when search term contains a space #1108

Merged

Conversation

landonb
Copy link
Contributor

@landonb landonb commented May 12, 2021

I found that set ignore-case = smart-case does not work when searching a term that includes the space character, e.g., searching 'foo bar' does not match 'Foo Bar'.

  • This is because the category for space is UTF8PROC_CATEGORY_ZS, which has the value 23, aka 0x17, or 0b10111, and the smart-case test, utf8_string_contains_uppercase, which calls utf8_string_contains, is using bitwise AND to check the category:

     if (property->category & category)
    

    So then the space character -- category 0x17 -- matches the uppercase category, UTF8PROC_CATEGORY_LU = 1. I.e., 0x17 & 1 is true.

  • My solution is to change the bitwise AND to the equality operator, ==.

    This is based on my understanding that the utf8proc categories are not bitmask values (and that utf8proc_get_property returns a single value, and not a bitmask), although I could be mistaken (this is my first time examining the utf8proc library).

  • This regression occurred in 29524d0.

Oh, and by the way, tig is such a delight to use! I discovered it when I got a MacBook for work and found out how slow gitk runs outside of Linux, and I haven't used gitk since -- tig's gotta be one of my favorite developer tools. Great work!!

I.e., `set ignore-case = smart-case` does not work when searching
a term that includes the space character, e.g., searching 'foo bar'
does not match 'Foo Bar'.
@koutcher
Copy link
Collaborator

LGTM, thanks.

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