Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

search: fix human string representation for metachars in query#44917

Merged
rvantonder merged 1 commit into
mainfrom
rvt/fix-human
Nov 29, 2022
Merged

search: fix human string representation for metachars in query#44917
rvantonder merged 1 commit into
mainfrom
rvt/fix-human

Conversation

@rvantonder

@rvantonder rvantonder commented Nov 29, 2022

Copy link
Copy Markdown
Contributor

Fixes https://github.com/sourcegraph/sourcegraph/issues/44738

A query containing special chars (, ) that are unbalanced needs to be quoted/escaped when generating a human representation, because these characters can break the meaning of a well-formed query with expressions.

See also https://sourcegraph.slack.com/archives/C014ZCKMCAV/p1669114578350879

Test plan

Added test

@cla-bot cla-bot Bot added the cla-signed label Nov 29, 2022
@sourcegraph-bot

sourcegraph-bot commented Nov 29, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff a01103d...e15634b.

Notify File(s)
@beyang internal/search/query/printer.go
internal/search/query/printer_test.go
internal/search/query/testdata/TestStringHuman/printer#17.golden
@camdencheek internal/search/query/printer.go
internal/search/query/printer_test.go
internal/search/query/testdata/TestStringHuman/printer#17.golden
@keegancsmith internal/search/query/printer.go
internal/search/query/printer_test.go
internal/search/query/testdata/TestStringHuman/printer#17.golden

@rvantonder

Copy link
Copy Markdown
Contributor Author

@chwarwick could you check the changelog entry for accuracy?

@rvantonder rvantonder marked this pull request as draft November 29, 2022 20:19
@rvantonder

Copy link
Copy Markdown
Contributor Author

Need to fix other things this affects, will put up for review again when ready.

@chwarwick

Copy link
Copy Markdown
Contributor

@chwarwick could you check the changelog entry for accuracy?

Change log looks good to me.

@rvantonder rvantonder marked this pull request as ready for review November 29, 2022 21:53
@rvantonder

Copy link
Copy Markdown
Contributor Author

Fixed up, ready for review

Comment thread CHANGELOG.md Outdated
@rvantonder rvantonder merged commit a40cf06 into main Nov 29, 2022
@rvantonder rvantonder deleted the rvt/fix-human branch November 29, 2022 23:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StringHuman causes query to change meaning for some searches

4 participants