Skip to content

add search function with more parameters#69

Merged
Firstyear merged 3 commits intokanidm:masterfrom
lperlaki-contrib:search-with-options
Feb 24, 2026
Merged

add search function with more parameters#69
Firstyear merged 3 commits intokanidm:masterfrom
lperlaki-contrib:search-with-options

Conversation

@lperlaki
Copy link
Copy Markdown
Contributor

expose the scope and attrs parameter for search in search_with function

Comment thread client/src/search.rs Outdated

pub async fn search<S: Into<String>>(
&mut self,
basedn: S,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You could use the same impl Into<String> here, we don't otherwise use S

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes we could do that but that would technically be a breaking change. And I don't now what the policy around that for this project is.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it really be a breaking change? What code would stop compiling? AFAIU this is equivalent syntactic sugar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code that uses that explicitly uses that generic parameter like search::<String> would stop to compile

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fair enough. That's a bit of an edge case, but it is indeed a breaking change

Copy link
Copy Markdown
Member

@Firstyear Firstyear left a comment

Choose a reason for hiding this comment

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

I think this could get messy pretty fast if we keep adding parameters. I think perhaps we could consider a search builder pattern?

@lperlaki
Copy link
Copy Markdown
Contributor Author

I think this could get messy pretty fast if we keep adding parameters. I think perhaps we could consider a search builder pattern?

yes I was thinking the same thing! in the latest commit the implementation is changed to the a builder pattern.
please let me know what you think about the design.
and for most use cases this new implementation is not a breaking change!

@lperlaki lperlaki requested a review from Firstyear February 23, 2026 11:58
@Firstyear Firstyear merged commit 4afa130 into kanidm:master Feb 24, 2026
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.

3 participants