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

gitserver: Add option to allow setting custom context line count#63840

Merged
eseliger merged 1 commit into
mainfrom
es/07-16-gitserveraddoptiontoallowsettingcustomcontextlinecount
Jul 19, 2024
Merged

gitserver: Add option to allow setting custom context line count#63840
eseliger merged 1 commit into
mainfrom
es/07-16-gitserveraddoptiontoallowsettingcustomcontextlinecount

Conversation

@eseliger

@eseliger eseliger commented Jul 16, 2024

Copy link
Copy Markdown
Member

This PR adds support for two new arguments in the gitserver API: InterHunkContext and ContextLines.
See the inline docstring for the two, but basically this allows to request diffs with a custom amount of context lines and hunk merging.

The defaults should be unchanged at 3 for both, according to https://sourcegraph.com/github.com/git/git/-/blob/diff.c?L60.

Closes SRC-467

Test plan:

Added unit tests.

@cla-bot cla-bot Bot added the cla-signed label Jul 16, 2024
@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels Jul 16, 2024

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @eseliger and the rest of your teammates on Graphite Graphite

Comment on lines 103 to 111

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is what you want, right?

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.

Yes, exactly

@eseliger eseliger force-pushed the es/07-16-gitserveraddoptiontoallowsettingcustomcontextlinecount branch from de4ffc1 to 8c77e31 Compare July 16, 2024 05:43
Comment thread internal/gitserver/commands_test.go Outdated
Comment on lines 831 to 850

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here's example go-diff output for the hunk (which we need to work with because sub repo perms enforcement)

@eseliger eseliger force-pushed the es/07-16-gitserveraddoptiontoallowsettingcustomcontextlinecount branch from 8c77e31 to 90bf1d2 Compare July 16, 2024 05:45
This PR adds support for two new arguments in the gitserver API: InterHunkContext and ContextLines.
See the inline docstring for the two, but basically this allows to request diffs with a custom amount of context lines and hunk merging.

The defaults should be unchanged at `3` for both, according to https://sourcegraph.com/github.com/git/git/-/blob/diff.c?L60.

Test plan:

Added unit tests.
@eseliger eseliger force-pushed the es/07-16-gitserveraddoptiontoallowsettingcustomcontextlinecount branch from 90bf1d2 to f5757d3 Compare July 16, 2024 06:11

@kritzcreek kritzcreek 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.

LGTM. Thank you!

Comment on lines 103 to 111

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.

Yes, exactly

@eseliger eseliger marked this pull request as ready for review July 16, 2024 07:32
@eseliger eseliger requested a review from a team July 16, 2024 07:32
@kritzcreek

Copy link
Copy Markdown
Contributor

Fun, when trying to use this I came across this totally not brittle mock that'll need updating as well. Unless there's a better gitClient fake I could use for testing?

https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph@2d76833cc7192540d3dc0461925869585a6123e0/-/blob/internal/gitserver/client.go?L150-158

@eseliger

Copy link
Copy Markdown
Member Author

Meh that thing should be gone by now. Instead, create a mock client like this:

diff := []byte(`yourmockdiffhere`)
	gs := gitserver.NewMockClient()
	gs.DiffFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, do gitserver.DiffOptions) (*gitserver.DiffFileIterator, error) {
		return gitserver.NewDiffFileIterator(io.NopCloser(bytes.NewReader(diff))), nil
	})

and pass that gitserver client to the function you're testing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants