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

search frontend: warn on structural search without lang: filter#43835

Merged
rvantonder merged 2 commits into
mainfrom
rvt/ss-lang-diag
Nov 3, 2022
Merged

search frontend: warn on structural search without lang: filter#43835
rvantonder merged 2 commits into
mainfrom
rvt/ss-lang-diag

Conversation

@rvantonder

@rvantonder rvantonder commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

This PR adds a warning diagnostic when structural search is active but lang: is not set.

When structural search is active users should add a lang: filter so that we can pick the right parser. If lang: isn't set, we may fall back to a generic parser, which can "miss" results that the user would normally expect.

More context: https://sourcegraph.slack.com/archives/C07KZF47K/p1667385583731439

Test plan

Updated unit tests.

App preview:

Check out the client app preview documentation to learn more.

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

sourcegraph-bot commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff e711dd7...beaf3ee.

Notify File(s)
@fkling client/shared/src/search/query/diagnostics.test.ts
client/shared/src/search/query/diagnostics.ts

@rvantonder rvantonder force-pushed the rvt/ss-lang-diag branch 2 times, most recently from 89f1dd3 to f26cdd0 Compare November 2, 2022 23:12

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.

Just adding lang:go explicitly to these existing test so we can isolate the diagnostics being tested.

@rvantonder rvantonder requested review from a team, fkling and limitedmage November 2, 2022 23:12
@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Nov 2, 2022

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.00% (+0.53 kb) 0.00% (+0.53 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits beaf3ee and 7b2e149 or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@camdencheek camdencheek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, I've definitely been caught by this before

Comment thread client/shared/src/search/query/diagnostics.ts Outdated
@rvantonder rvantonder enabled auto-merge (squash) November 3, 2022 19:30
@rvantonder rvantonder disabled auto-merge November 3, 2022 19:30
@rvantonder rvantonder enabled auto-merge (squash) November 3, 2022 20:57
@rvantonder rvantonder merged commit bde3ece into main Nov 3, 2022
@rvantonder rvantonder deleted the rvt/ss-lang-diag branch November 3, 2022 21:08
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.

5 participants