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

cody web: do not include ignored files in context#59907

Merged
taras-yemets merged 14 commits into
mainfrom
ty/cody-ignore-web
Feb 2, 2024
Merged

cody web: do not include ignored files in context#59907
taras-yemets merged 14 commits into
mainfrom
ty/cody-ignore-web

Conversation

@taras-yemets

@taras-yemets taras-yemets commented Jan 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Excludes files from Cody scope if they match patterns from the Cody ignore file.

Ignored file content 👇🏻
Screenshot 2024-01-29 at 15 55 53

Ignored Chat Transcript
✅ Screenshot 2024-01-29 at 15 54 51 Screenshot 2024-01-29 at 16 00 20
🚫 Screenshot 2024-01-29 at 15 55 25 Screenshot 2024-01-29 at 15 57 15

Note

Known issue: Cody chat refers to the previously opened file in the current transcript if the current file is excluded from the scope (https://github.com/sourcegraph/cody/issues/2925). FYI when testing this PR.

Test plan

  • Open repo which has .cody/ignore file
  • Open a file matching the pattern from the ignore file
  • View chat transcript (e.g., network tab) and ensure that code snippets from the ignored file are not included

@cla-bot cla-bot Bot added the cla-signed label Jan 26, 2024
@cbart cbart mentioned this pull request Jan 29, 2024
Closed
4 tasks
@taras-yemets taras-yemets marked this pull request as ready for review January 29, 2024 14:12
@taras-yemets taras-yemets requested review from a team and 0xnmn January 29, 2024 14:12
cbart
cbart approved these changes Jan 29, 2024
View reviewed changes
Comment thread client/web/src/cody/components/ScopeSelector/RepositoriesSelectorPopover.tsx Outdated
0xnmn
0xnmn reviewed Jan 29, 2024
View reviewed changes

@0xnmn 0xnmn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see we are using useCodyIgnore only to check the currently opened file path. Have we already hooked the same check at backend for files returned on fetching context using getCodyContext?

Comment thread client/web/src/cody/components/ChatUI/useCodyIgnore.ts Outdated
@taras-yemets

taras-yemets commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

Have we already hooked the same check at backend for files returned on fetching context using getCodyContext?

cc: @thenamankumar

@chwarwick

chwarwick commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

I see we are using useCodyIgnore only to check the currently opened file path. Have we already hooked the same check at backend for files returned on fetching context using getCodyContext?

@thenamankumar I'm working on updating the getCodyContext api currently.

@taras-yemets

taras-yemets commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

@chwarwick, is the current plan to put checking the ignore file behind the feature flag (https://github.com/sourcegraph/sourcegraph/pull/59836/files#r1465507213)?

@chwarwick

chwarwick commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

@chwarwick, is the current plan to put checking the ignore file behind the feature flag (https://github.com/sourcegraph/sourcegraph/pull/59836/files#r1465507213)?

Yes I was going to add an experimental setting for it since that aligns with vs code currently.

@taras-yemets

taras-yemets commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

Yes I was going to add an experimental setting for it since that aligns with vs code currently.

@chwarwick, we should probably respect this setting on the client too. I'll hold on merging this PR until yours is merged.

chwarwick
chwarwick approved these changes Feb 2, 2024
View reviewed changes
Comment thread CHANGELOG.md Outdated
@taras-yemets
Update CHANGELOG.md
1d772fd 
Co-authored-by: Chris Warwick <christopher.warwick@sourcegraph.com>
@taras-yemets taras-yemets merged commit daf29d1 into main Feb 2, 2024
@taras-yemets taras-yemets deleted the ty/cody-ignore-web branch February 2, 2024 15:07
@taras-yemets taras-yemets linked an issue Feb 6, 2024 that may be closed by this pull request
Cody Web respects codyignore #59681
Closed
@cbart cbart mentioned this pull request Feb 6, 2024
Closed
sourcegraph-release-bot pushed a commit that referenced this pull request Feb 7, 2024
@taras-yemets @github-actions
cody web: do not include ignored files in context (#59907)
8dbd957 
(cherry picked from commit daf29d1)
@sourcegraph-release-bot sourcegraph-release-bot mentioned this pull request Feb 7, 2024
Merged
keegancsmith added a commit that referenced this pull request Feb 7, 2024
@sourcegraph-release-bot @taras-yemets @keegancsmith
[Backport 5.3] cody web: do not include ignored files in context (#60256
8a2b0bf 
)

cody web: do not include ignored files in context (#59907)

(cherry picked from commit daf29d1)

Co-authored-by: Taras Yemets <yemets.taras@gmail.com>
Co-authored-by: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@cbart cbart cbart approved these changes

@chwarwick chwarwick chwarwick approved these changes

@arafatkatze arafatkatze arafatkatze approved these changes

@0xnmn 0xnmn 0xnmn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport 5.3 cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cody Web respects codyignore

5 participants

@taras-yemets @chwarwick @cbart @arafatkatze @0xnmn