Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure config.path = None and config.path missing mean the same thing #704

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ianawilson
Copy link

@ianawilson ianawilson commented May 1, 2024

Previously if path was present, it was expected to have a value. This allows path to be None, and for None and empty to act in the same way.

Originally I made this change because setting defaults to False and keyword to True didn't seem to be enough for me.

@davidmezzetti
Copy link
Member

Just for my clarity, the PR supports adding a a config key of path: None? The current method assumes path wouldn't be there but this change takes it a step further and checks that if the key is there, it has a value?

@ianawilson ianawilson changed the title Allow config.path = None and config.keyword = True for full text search only Ensure config.path = None and config.path missing mean the same thing May 14, 2024
@ianawilson
Copy link
Author

Just for my clarity, the PR supports adding a a config key of path: None? The current method assumes path wouldn't be there but this change takes it a step further and checks that if the key is there, it has a value?

We talked a bit in community slack, but the gist is that I wanted None and missing to act the same. The title and description have been updated for clarity.

@ianawilson ianawilson marked this pull request as ready for review May 14, 2024 17:52
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.

None yet

2 participants