[FEATURE] Add option -r/--regex to find #3083

carlsmedstad · 2025-03-09T19:25:24Z

If the option -r/--regex is supplied to gopass find, the argument is treated as a regex pattern instead of a substring.

If the option -r/--regex is supplied to 'gopass find', the argument is treated as a regex pattern instead of a substring. Signed-off-by: Carl Smedstad <carl.smedstad@protonmail.com>

gemini-code-assist

Hello @carlsmedstad, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request introduces a new feature to the gopass find command, allowing users to treat the search argument as a regular expression by supplying the -r/--regex option. This provides more flexible and powerful search capabilities within gopass. The changes include adding a new flag to the command, modifying the filter function to handle regex matching, and adding a new test case to verify the regex functionality.

Highlights

Feature Addition: Adds the -r/--regex option to the gopass find command.
Regex Implementation: Implements regular expression matching in the filter function.
Testing: Adds a new test case to verify the regex matching functionality.

Changelog

internal/action/commands.go
- Added a new boolean flag --regex (alias -r) to the find command to enable regex matching.
internal/action/find.go
- Imported the regexp package.
- Modified the find function to pass the regex flag value to the filter function.
- Modified the filter function to perform regex matching if the regex flag is enabled. If the regex flag is not enabled, the function defaults to substring matching.
internal/action/find_test.go
- Added a new test case to verify the regex matching functionality, including cases with and without matches.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Trivia time!

What is the origin of the term 'regex'?

Click here for the answer

The term 'regex' is short for 'regular expression', which comes from the mathematical theory of formal languages. Regular expressions were formalized in the 1950s by mathematician Stephen Kleene.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new -r/--regex option to the gopass find command, allowing users to treat the search argument as a regular expression. The changes are well-structured and include a new test case to verify the functionality. However, there are a few areas that could be improved for clarity and robustness.

Summary of Findings

Error Handling in filter function: The filter function currently ignores errors returned by regexp.MatchString. While the likelihood of an error is low, it's important to handle potential errors to prevent unexpected behavior.
Case Sensitivity in Regex Matching: The regex matching in the filter function is case-sensitive. Consider adding an option or modifying the regex to support case-insensitive matching if desired.

Merge Readiness

The pull request is almost ready for merging. Addressing the error handling in the filter function would improve the robustness of the code. Additionally, consider whether case-insensitive regex matching should be supported. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.

gemini-code-assist · 2025-03-09T19:25:39Z

internal/action/find.go

+			matched, err := regexp.MatchString(needle, value)
+			if err == nil && matched {
+				choices = append(choices, value)


The code currently ignores the error returned by regexp.MatchString. While regex compilation errors are unlikely at runtime, it's best practice to handle them. Consider logging the error or returning it to the caller to prevent unexpected behavior.

matched, err := regexp.MatchString(needle, value) if err != nil { // Log the error or return it to the caller fmt.Printf("Regex match error: %v\n", err) continue // Or return an error } if matched {

Reviewed by an AI, that's a first for me :D

Addressed in 2aa5f7a.

For example: $ ./gopass find -r 'mystring[' Error: error parsing regexp: missing closing ]: `[` Signed-off-by: Carl Smedstad <carl.smedstad@protonmail.com>

dominikschulz

Good idea, thank you. Please address my feedback and then we're good to go.

dominikschulz · 2025-03-10T10:20:26Z

internal/action/find.go

+			if matched {
+				choices = append(choices, value)
+			}
+		} else if strings.Contains(strings.ToLower(value), strings.ToLower(needle)) {


else if is highly unusual in Go and at least in a loop rarely needed. I would suggest to handle the regexp case and at the end of that block add a continue so the non-regexp part is skipped in that case.

May if would make sense so swap the order of both cases, but I'm sure about that.

dominikschulz · 2025-03-10T10:21:55Z

internal/action/find.go

@@ -141,13 +144,21 @@ func (s *Action) findSelection(ctx context.Context, c *cli.Context, choices []st
 	}
 }

-func filter(l []string, needle string) []string {
+func filter(l []string, needle string, regex bool) ([]string, error) {


The variables regex and the package regexp are a little bit confusing. Maybe you could name the parameter reMatch or something like that?

Good point, renamed in 0a129b9.

dominikschulz · 2025-03-10T10:23:22Z

internal/action/find.go

 	choices := make([]string, 0, 10)
 	for _, value := range l {
-		if strings.Contains(strings.ToLower(value), needle) {
+		if regex {
+			matched, err := regexp.MatchString(needle, value)


regexp.MatchString will compile the regular expression in every cycle of the loop. That could be pretty expensive. Please compile the RE outside of the loop and then use the compiled RE inside the loop. This will also simplify error handling.

Compiling outside of the loop in: ecc5704

Also replaced the if else with an early return.

Thanks for the good feedback - My Go experience is very limited :) Would you like me to squash it all?

[FEATURE] Add option -r/--regex to find

aa31685

If the option -r/--regex is supplied to 'gopass find', the argument is treated as a regex pattern instead of a substring. Signed-off-by: Carl Smedstad <carl.smedstad@protonmail.com>

gemini-code-assist bot reviewed Mar 9, 2025

View reviewed changes

[UX] Fail and produce error message when bad regex supplied to find -r

2aa5f7a

For example: $ ./gopass find -r 'mystring[' Error: error parsing regexp: missing closing ]: `[` Signed-off-by: Carl Smedstad <carl.smedstad@protonmail.com>

dominikschulz requested changes Mar 10, 2025

View reviewed changes

carlsmedstad added 2 commits March 12, 2025 18:25

[CLEANUP] Rename argument regex to reMatch

0a129b9

[CLEANUP] Compile regex outside of loop

ecc5704

dominikschulz approved these changes Mar 12, 2025

View reviewed changes

dominikschulz merged commit 4a9f658 into gopasspw:master Mar 12, 2025
5 of 8 checks passed

carlsmedstad deleted the find-regex branch March 14, 2025 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[FEATURE] Add option -r/--regex to find #3083

[FEATURE] Add option -r/--regex to find #3083

carlsmedstad commented Mar 9, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 9, 2025

Uh oh!

carlsmedstad Mar 9, 2025

Uh oh!

dominikschulz left a comment

Uh oh!

dominikschulz Mar 10, 2025

Uh oh!

dominikschulz Mar 10, 2025

Uh oh!

carlsmedstad Mar 12, 2025

Uh oh!

dominikschulz Mar 10, 2025

Uh oh!

carlsmedstad Mar 12, 2025

Uh oh!

carlsmedstad Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[FEATURE] Add option -r/--regex to find #3083

[FEATURE] Add option -r/--regex to find #3083

Conversation

carlsmedstad commented Mar 9, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

carlsmedstad Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

dominikschulz left a comment

Choose a reason for hiding this comment

Uh oh!

dominikschulz Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

dominikschulz Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

carlsmedstad Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

dominikschulz Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

carlsmedstad Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

carlsmedstad Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!