(Bug report + fix) gitk "IgnCase" search doesn't ignore case

Juan Navarro· Jun 14, 2018, 12:53 UTC · lore

Hi,

this question was originally posted on the Google Groups list, trying to get help (https://groups.google.com/d/topic/git-users/QAFKOQU4eUo/discussion). Now that it's confirmed as a bug and I have a proposed solution, I'm posting here.

Gitk "find commit" search function doesn't follow the "IgnCase" option that is selectable with a combo selector on the right side of the window; it should be searching in a case-insensitive way, but it doesn't.

Steps to reproduce:
1. Clone any repo. I'm right now using 
https://github.com/Kurento/kms-core.git 
<https://github.com/Kurento/kms-core.git>
2. cd into the repo and launch gitk
3. In the "Find commit" bar, select "changing lines matching"
4. In the right side of the same bar, select "IgnCase"
5. Search for a term that you know exists in uppercase, but write the 
search in lowercase. In my example, I'm searching for "leaky_time".
6. No search results appear.
7. Change your search word to uppercase.
8. Some search results appear, thus proving that the search is being 
done case-sensitive.

The command that gets called when searching is in lines 4804 & 4809 (https://github.com/git/git/blob/master/gitk-git/gitk#L4804 <https://github.com/git/git/blob/master/gitk-git/gitk#L4804>): git diff-tree -r -s --stdin -G<SearchTerm>

Proposed solution is almost trivial: check if the "IgnCase" option is enabled, and in that case add the flag "-i" to the git command. Now that we are at it, it's probably correct to add that option to all search modes. A diff is attached to this email, hoping that someone is able to apply it (sorry I'm not familiarized with contributing with patch files, but the diff is pretty small anyways).

Regards, Juan

4809c4809,4815 < set cmd [concat | git diff-tree -r -s --stdin $gdtargs] ---

Show 7 quoted lines

> 
>     set igncase_arg ""
>     if {$findtype eq [mc "IgnCase"]} {
>         set igncase_arg "-i"
>     }
> 
>     set cmd [concat | git diff-tree $igncase_arg -r -s --stdin $gdtargs]

Eric Sunshine· Jun 14, 2018, 21:44 UTC · re: Juan Navarro · lore

Re: (Bug report + fix) gitk "IgnCase" search doesn't ignore case

On Thu, Jun 14, 2018 at 02:53:03PM +0200, Juan Navarro wrote:

Show 16 quoted lines

> Gitk "find commit" search function doesn't follow the "IgnCase" option that
> is selectable with a combo selector on the right side of the window; it
> should be searching in a case-insensitive way, but it doesn't.
> 
> Steps to reproduce:
> [...]
> 3. In the "Find commit" bar, select "changing lines matching"
> 4. In the right side of the same bar, select "IgnCase"
> [...]
> 
> Proposed solution is almost trivial: check if the "IgnCase" option is
> enabled, and in that case add the flag "-i" to the git command. Now that we
> are at it, it's probably correct to add that option to all search modes.
> A diff is attached to this email, hoping that someone is able to apply it
> (sorry I'm not familiarized with contributing with patch files, but the diff
> is pretty small anyways).

Thanks for reporting this.

A different way to interpret the situation is that the user-interface is being inconsistent. For instance, the "fields" pop-up next to the "exact/ignore-case/regexp" pop-up does not seem to make sense for search types other than "containing", so it probably ought to be disabled for anything other than "containing". By extension, one could argue that the "exact/ignore-case/regexp" pop-up also ought be disabled for non-"containing" searches. The fact that they are not disabled confuses people into thinking that they should be functional for all searches, not just for "containing" searches, even though such functionality was never implemented (and indeed, may be difficult to implement fully).

Your proposed fix handles only the "ignore case" item; it does not implement "regexp" functionality, so it could be considered incomplete. A more complete fix would also disable the "regexp" item to avoid misleading users, and to head off future bug reports similar to this one saying that "regexp" doesn't work for non-"containing" searches. (Bonus points for also disabling the "fields" pop-up for non-"containing" searches when it's not applicable.)

Below is your fix wrapped up as a proper patch and sent in-line rather than as an attachment. It's also slightly simplified by dropping the unnecessary extra variable. You'll need to sign-off on the patch if it is ever to be accepted. You can do so by adding it after my sign-off. If you don't feel like re-sending the patch with your sign-off, you can alternately reply to this email with a "Signed-off-by: Your Name <your@email>" line.

Note, however, that the gitk project is, at best, deeply slumbering, so it's not clear when or even if patches will be incorporated. (Indeed, other recent gitk-related patches sent to the mailing list have not yet been picked up.)

--- >8 ---
From: Juan Navarro <juan.navarro@gmx.es>
Subject: [PATCH] gitk: support "ignore case" for non-"containing" searches

The "Exact/Ignore Case/Regexp" pop-up control only affects "containing" searches. Other types of searches ("touching paths", "adding/removing strings", "changing lines matching") ignore this option. Improve the user experience by also recognizing "ignore case" for non-"containing" searches.

Note: This change only implements the "ignore case" option for these
other search types; it does not add support for the "regexp" option
(which still only affects "containing" searches). A more complete "fix"
would improve the user experience even more by making the UI more
consistent; namely, by disabling options which don't make sense or are
not easily implemented for non-"containing" searches. In particular, the
"regexp" pop-up item and the neighboring "fields" pop-up control ought
perhaps be disabled when a non-"containing" search is selected.

[es: wrote commit message; slightly simplified proposed "fix"]

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a14d7a16b2..fbb75f7390 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4806,6 +4806,9 @@ proc do_file_hl {serial} {
 	# must be "containing:", i.e. we're searching commit info
 	return
     }
+    if {$findtype eq [mc "IgnCase"]} {
+	set gdtargs [linsert $gdtargs 0 "-i"]
+    }
     set cmd [concat | git diff-tree -r -s --stdin $gdtargs]
     set filehighlight [open $cmd r+]
     fconfigure $filehighlight -blocking 0

-- 
2.18.0.rc1.256.g331a1db143