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

Context: return lines around symbol match#63773

Merged
jtibshirani merged 1 commit intomainfrom
jtibs/symbols
Jul 11, 2024
Merged

Context: return lines around symbol match#63773
jtibshirani merged 1 commit intomainfrom
jtibs/symbols

Conversation

@jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Jul 10, 2024

This PR fixes an important bug in #62976, where we didn't properly map the
symbol line match to the return type. Instead, we accidentally treated symbol
matches like file matches and returned the start of the file.

Test plan

Add new unit test for symbol match conversion. Extensive manual testing.

@cla-bot cla-bot bot added the cla-signed label Jul 10, 2024
@github-actions github-actions bot added team/product-platform team/search-platform Issues owned by the search platform team labels Jul 10, 2024
@jtibshirani jtibshirani requested a review from a team July 11, 2024 00:04
@jtibshirani jtibshirani marked this pull request as ready for review July 11, 2024 00:06
func fileMatchToContextMatch(fm *result.FileMatch) FileChunkContext {
if len(fm.ChunkMatches) == 0 {
var startLine int
if len(fm.Symbols) != 0 {
Copy link
Member

@stefanhengl stefanhengl Jul 11, 2024

Choose a reason for hiding this comment

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

Just a thought. In the code I see that we do this kind of check in several places. I wonder whether it would help to just make this a method of FileMatch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, what are you suggesting we could pull out? A method like containsSymbols? Or something like firstStartLine?

@jtibshirani
Copy link
Contributor Author

I'm going to merge but happy to make any follow-ups.

@jtibshirani jtibshirani merged commit 004eb0f into main Jul 11, 2024
@jtibshirani jtibshirani deleted the jtibs/symbols branch July 11, 2024 15:47
sourcegraph-release-bot pushed a commit that referenced this pull request Jul 11, 2024
This PR fixes an important bug in #62976, where we didn't properly map the
symbol line match to the return type. Instead, we accidentally treated symbol
matches like file matches and returned the start of the file.

## Test plan

Add new unit test for symbol match conversion. Extensive manual testing.

(cherry picked from commit 004eb0f)
@sourcegraph-release-bot
Copy link
Collaborator

The backport to 5.5.x failed at https://github.com/sourcegraph/sourcegraph/actions/runs/9894557194:

Server Error

To backport this PR manually, you can either:

Via the sg tool

Use the sg backport command to backport your commit to the release branch.

sg backport -r 5.5.x -p 63773
Via your terminal

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.5.x 5.5.x
# Navigate to the new working tree
cd .worktrees/backport-5.5.x
# Create a new branch
git switch --create backport-63773-to-5.5.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 004eb0fd830755376c8ee6b895a814d79bd8f21b
# Push it to GitHub
git push --set-upstream origin backport-63773-to-5.5.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.5.x

If you encouter conflict, first resolve the conflict and stage all files, then run the commands below:

git cherry-pick --continue
# Push it to GitHub
git push --set-upstream origin backport-63773-to-5.5.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.5.x
  • Follow above instructions to backport the commit.
  • Create a pull request where the base branch is 5.5.x and the compare/head branch is backport-63773-to-5.5.x., click here to create the pull request.

Once the pull request has been created, please ensure the following:

  • Make sure to tag @sourcegraph/release in the pull request description.

  • kindly remove the release-blocker from this pull request.

@sourcegraph-release-bot sourcegraph-release-bot added backports release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases failed-backport-to-5.5.x labels Jul 11, 2024
jdpleiness pushed a commit that referenced this pull request Jul 11, 2024
This PR fixes an important bug in #62976, where we didn't properly
map the
symbol line match to the return type. Instead, we accidentally treated
symbol
matches like file matches and returned the start of the file.

## Test plan

Add new unit test for symbol match conversion. Extensive manual testing.
<br> Backport 004eb0f from #63773

Co-authored-by: Julie Tibshirani <[email protected]>
@jtibshirani jtibshirani removed release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases failed-backport-to-5.5.x labels Jul 11, 2024
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.

3 participants