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

bug: files in bin/ are filtered out of @-mention in chat #2401

Closed
DanTup opened this issue Dec 15, 2023 · 9 comments · Fixed by #2472
Closed

bug: files in bin/ are filtered out of @-mention in chat #2401

DanTup opened this issue Dec 15, 2023 · 9 comments · Fixed by #2472
Assignees
Labels
bug Something isn't working clients/vscode

Comments

@DanTup
Copy link
Contributor

DanTup commented Dec 15, 2023

Version

main 2a00da0

Describe the bug

When I typed @, my open file bin\\main.dart was already in the results, but as I started typing bin it vanished.

bad_filter.mp4

Expected behavior

Files that exist and match my query should not be filtered out.

Additional context

No response

@DanTup DanTup added bug Something isn't working clients/vscode labels Dec 15, 2023
@DanTup DanTup self-assigned this Dec 15, 2023
@DanTup
Copy link
Contributor Author

DanTup commented Dec 15, 2023

This turned out to be very specific... The default exclude list includes bin/:

const fileExcludesPattern = '**/{*.env,.git,out/,dist/,bin/,snap,node_modules,__pycache__}**'

This is something I've hit before... When I did .NET, I would have bin/ in by global gitignore (because in .NET it's compiled binaries). It's quite common in Dart to use bin/ for the entry points of packages:

https://dart.dev/tools/pub/package-layout#:~:text=make_lunch.dart%0A%20%20bin/-,enchilada,-doc/%0A%20%20%20%20api/%20***%0A%20%20%20%20getting_started

@toolmantim any thoughts on this (since you picked the original defaults)? I'm not familiar with how many languages use bin/.. For .NET, probably explicitly including bin/Debug/ and bin/Release/ would filter out the garbage, although perhaps the real fix here is to use gitignore/etc.

@DanTup DanTup changed the title bug: existing files are unexpectedly filtered out of @-mention in chat bug: files in bin/ are filtered out of @-mention in chat Dec 15, 2023
@toolmantim
Copy link
Contributor

Best to ask @abeatrix when she’s back, or @beyang — unsure how that initial exclude list was chosen.

@abeatrix
Copy link
Contributor

We can just remove the keyword from the exclude list if the keyword is included in the exclude list, or just remove the exclude list in general from the @ search if you want. I was just using the same exclude list I used for the generate unit test to filter out uncommon files for unit tests and thought the same list could be used, no other particular reasons.

@abeatrix
Copy link
Contributor

I'd recommend keeping it and remove it only if the keyword is included because those files will always show up on top of the list from those folder compared to the actual ones with the same names

@DanTup
Copy link
Contributor Author

DanTup commented Dec 17, 2023

It may look odd if the user is typing @bin/main.dart if it's missing from the list until they've typed @bin/?

I wonder if it should be included (not in the exclude list) but penalised in the ranking unless the query starts with it?

That way if your project is small, it won't be obvious bin/main.dart is missing, but it may be further down the list until you've typed bin/?

@toolmantim
Copy link
Contributor

The fuzzysort ranking algo is easy to tweak, I like the idea of just downscoring it.

@toolmantim
Copy link
Contributor

If we do keep the exclude list, we should probably make it less invisible and hard-coded into the source by moving it to a user setting with the default value. With a cog icon in the popover, similar to the chat window, that opens the settings filtered to this feature.

@DanTup
Copy link
Contributor Author

DanTup commented Dec 22, 2023

Maybe we should switch it to using the ignore file (#1382)? Seems pointless showing the user something in the dropdown to then filter it out later. (that was already done in #1382)

For now, I'll remove bin from the hard-coded ignore list and reduce the relevance.

@DanTup
Copy link
Contributor Author

DanTup commented Dec 22, 2023

I've opened #2472

bin.mp4

DanTup added a commit that referenced this issue Jan 5, 2024
Fixes #2401 by removing `bin/`
from the ignore list and instead just penalizing it unless you have
typed `bin` in the query specifically.

https://github.com/sourcegraph/cody/assets/1078012/2ad0ef3b-b2f9-45bd-bfc0-54fd77811511


Also applies ignore file to these files


## Test plan

- Run `pnpm test:unit` and ensure the new tests pass
- Open a project with a file in a folder named `bin` and ensure the file
shows up in search results, near the bottom unless you've explicitly
typed "bin"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working clients/vscode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants