Fix the issue Wildcard matching should be used in help file search #3971
Conversation
| // If the input is pattern instead of string, we need to use Regex expression. | ||
| if (pattern.Contains("*") || pattern.Contains("?")) | ||
|
|
||
| string fileName = Path.GetFileName(filePath); |
There was a problem hiding this comment.
fileName isn't used when wildcardPattern is null, so we should avoid the call (which copies and allocates memory) unless we'll actually use the value.
There was a problem hiding this comment.
Actually, shouldn't fileName also be used when there are no wildcards? Currently
if (filePath.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) >= 0)is searching the whole path for a match. If it is desirable to only target the file name when performing the wildcard match, shouldn't it also be appropriate here? (Actually, given that an exact match is expected — that's how GetFiles would work in the non-UNIX case for a non-wildcard pattern, if I am not mistaken — shouldn't the test be against fileName.Equals(pattern, StringComparison.OrdinalIgnoreCase) instead of using IndexOf?)
(An unrelated minor issue I believe exists with the logic of GetFiles is using break instead of continue when a non-wildcard match is found. This might limit the matches in the (rare) case in which a file name equals a wildcard search pattern, e.g. foo?, though I haven't tested how GetFileswould behave in that scenario, and it might be nitpicking if the expected directory and file names are curated.
Also, sorry for butting in, but since I wasn't sure enough about my observations I didn't want to open an issue, so I didn't know a better way to proceed.)
There was a problem hiding this comment.
@gonhidi - thanks for the extra scrutiny - it is definitely appreciated.
It does seem like there should be consistency between wildcards and non-wildcards. Feel free to open an issue.
|
@lzybkr Thanks Jason, your comment has been resolved |
Fix issue #3967
Summary of the issue:
According to Jason's suggestion, replace the regex expression with wildcard.
Root cause of the issue:
This is an code improvement. The existed test cases are covering the scenario