Skip to content

fix: glob matching case sensitivity#112

Merged
dmehala merged 1 commit intomainfrom
dmehala/fix-glob-case-sensitive
Apr 16, 2024
Merged

fix: glob matching case sensitivity#112
dmehala merged 1 commit intomainfrom
dmehala/fix-glob-case-sensitive

Conversation

@dmehala
Copy link
Copy Markdown
Collaborator

@dmehala dmehala commented Apr 13, 2024

Description

Ensure glob matching is case insensitive and add test covering case sensitivity.

Ensure glob matching is case insensitive and add test covering case
sensitivity.
@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Apr 13, 2024

Benchmarks

Benchmark execution time: 2024-04-13 18:20:34

Comparing candidate commit 74af266 in PR branch dmehala/fix-glob-case-sensitive with baseline commit b553930 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

@dmehala dmehala requested a review from Anilm3 April 15, 2024 17:03

while (p < pattern.size() || s < subject.size()) {
if (p < pattern.size()) {
const size_t p_size = pattern.size();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I can't comment on the relevant line, but instead of using Index = std::size_t; it would make more sense to use using Index = StringView::size_type;, it should work as expected with both absl::string_view and std::string_view. In practice it doesn't make a difference.

Copy link
Copy Markdown
Contributor

@Anilm3 Anilm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dmehala dmehala merged commit e576445 into main Apr 16, 2024
@dmehala dmehala deleted the dmehala/fix-glob-case-sensitive branch April 16, 2024 10:53
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