Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

SearchFilter.get_search_terms returns list.#9338

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
lovelydinosaur merged 1 commit intomasterfromget-search-terms-returns-list
Mar 22, 2024

Conversation

@lovelydinosaur
Copy link
Contributor

@lovelydinosaurlovelydinosaur commentedMar 22, 2024
edited
Loading

#9017 unintentionally treatedSearchFilter.get_search_terms as a private method, changing its signature from a list of strings, to a string.

This pull request reverts that change, by pulling the"search_smart_split" inside the method, ensuring that search terms are correctly tokenised by"quoted strings like this" andcomma,sperated,strings,like,this.

Closes#9308

Copy link
Contributor

@jthevosjthevos left a comment

Choose a reason for hiding this comment

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

🚢 🚢 🚢

@lovelydinosaurlovelydinosaur merged commiteb361d2 intomasterMar 22, 2024
@lovelydinosaurlovelydinosaur deleted the get-search-terms-returns-list branchMarch 22, 2024 10:52
@lovelydinosaurlovelydinosaur mentioned this pull requestMar 22, 2024


defsearch_smart_split(search_terms):
"""generator that first splits string by spaces, leaving quoted phrases together,

Choose a reason for hiding this comment

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

Came fromyour haiku
Just to add 🐼 comment
Docstring needs fix

(“generator” -> Create list)

jthevos reacted with heart emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll put up a PR for it!

lovelydinosaur reacted with thumbs up emojiyuwash reacted with heart emoji
field=CharField(trim_whitespace=False,allow_blank=True)
returnfield.run_validation(value)
cleaned_value=field.run_validation(value)
returnsearch_smart_split(cleaned_value)
Copy link
Contributor

Choose a reason for hiding this comment

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

This could have been done by simply changing to

returnlist(search_smart_split(cleaned_value))

There is no point to changing the return value and definition ofsearch_smart_split.

@sevdog
Copy link
Contributor

PS: an anti-regression test should be implemented to avoid this, since both older and new versions passed the same tests without any failure.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

3 more reviewers

@yuwashyuwashyuwash left review comments

@jthevosjthevosjthevos approved these changes

@sevdogsevdogsevdog left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3.15 backward compatibility issue with 3.14 -rest_framework.filters.SearchFilter.get_search_terms returnsstr instead oflist

4 participants

@lovelydinosaur@sevdog@yuwash@jthevos

[8]ページ先頭

©2009-2025 Movatter.jp