-
Notifications
You must be signed in to change notification settings - Fork 23
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
Create clickable links from the listed context files #297
Conversation
cdb86e4
to
a27efa3
Compare
a27efa3
to
cbed4ac
Compare
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍 I've requested some small changes (see: comments).
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/sourcegraph/cody/chat/ContextFilesMessage.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes.
So the remaining unknown seems to be the remote files. Take a look at the agent's response: It contains a doubled entry of the local file:
and two other entries with repo & revision:
I assume that the file referencing the local file should link it directly. What about the "remote files"? We can link the local files to them BUT there can be inconsistencies b/w local and remote (eg. the agent uses a file that exists in the remote but it does not exist locally). How should we handle it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not bump IJ platform version!
@danielmarquespt, we haven't consulted this change with you so please take a look on attached demo in first comment & eventually confirm. The design has slightly changed (paths now have "link" color + no border/outline). |
cbbe1ff
to
ecc9646
Compare
it should be all good now |
ecc9646
to
25a44b4
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I should have tested it in the scope of: #297. Anyway, it fixes the links in Windows. ## Test plan - [x] Test on Windows + MacOS - [x] run command/ prompt chat & see the files - [x] files should be clickable and navigate correctly
Fixes #255
Test plan
:runIde
Note that we have issue with unrelated files shown in the context read files list: #298