feat: [CODE-3129]: add ignore whitespace for diff (#3327)

* better parsing of query param ignore_whitespace
* replace hideWhitespace with ignoreWhitespace
* replace rest client literals with vars
* added openapi
* add ignore whitespace for diff
pull/3616/head
Enver Biševac 2025-01-31 21:48:57 +00:00 committed by Harness
parent 2f9d9583e6
commit 92e39dee79
13 changed files with 261 additions and 56 deletions

28
.testapi/diff.http Normal file
View File

@ -0,0 +1,28 @@
GET {{baseurl}}/repos/root/{{repo}}/+/diff/{{targetBranch}}...{{sourceBranch}}
Accept: text/plain
Authorization: {{token}}
### Get diff ignore white space
GET {{baseurl}}/repos/root/{{repo}}/+/diff/{{targetBranch}}...{{sourceBranch}}?ignore_whitespace=true
Accept: text/plain
Authorization: {{token}}
### Get diff with hidden white spaces
GET {{baseurl}}/repos/root/{{repo}}/+/commits/{{commit}}/diff
Accept: text/plain
Authorization: {{token}}
### Get commit diff ignore white space
GET {{baseurl}}/repos/root/{{repo}}/+/commits/{{commit}}/diff?ignore_whitespace=true
Accept: text/plain
Authorization: {{token}}
### Get diff stats
GET {{baseurl}}/repos/root/{{repo}}/+/diff-stats/{{targetBranch}}...{{sourceBranch}}
Accept: text/plain
Authorization: {{token}}
### Get diff stats ignore white space
GET {{baseurl}}/repos/root/{{repo}}/+/diff-stats/{{targetBranch}}...{{sourceBranch}}?ignore_whitespace=true
Accept: text/plain
Authorization: {{token}}

7
.testapi/login.http Normal file
View File

@ -0,0 +1,7 @@
POST {{baseurl}}/login
Content-Type: application/json
{
"login_identifier": "{{login_identifier}}",
"password": "{{password}}"
}

View File

@ -65,6 +65,7 @@ func (c *Controller) Diff(
pullreqNum int64,
setSHAs func(sourceSHA, mergeBaseSHA string),
includePatch bool,
ignoreWhitespace bool,
files ...gittypes.FileDiffRequest,
) (types.Stream[*git.FileDiff], error) {
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
@ -87,6 +88,7 @@ func (c *Controller) Diff(
HeadRef: pr.SourceSHA,
MergeBase: true,
IncludePatch: includePatch,
IgnoreWhitespace: ignoreWhitespace,
}, files...))
return reader, nil

View File

@ -34,6 +34,7 @@ func (c *Controller) RawDiff(
session *auth.Session,
repoRef string,
path string,
ignoreWhitespace bool,
files ...gittypes.FileDiffRequest,
) error {
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
@ -51,6 +52,7 @@ func (c *Controller) RawDiff(
BaseRef: info.BaseRef,
HeadRef: info.HeadRef,
MergeBase: info.MergeBase,
IgnoreWhitespace: ignoreWhitespace,
}, files...)
}
@ -59,6 +61,7 @@ func (c *Controller) CommitDiff(
session *auth.Session,
repoRef string,
rev string,
ignoreWhitespace bool,
w io.Writer,
) error {
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
@ -69,6 +72,7 @@ func (c *Controller) CommitDiff(
return c.git.CommitDiff(ctx, &git.GetCommitParams{
ReadParams: git.CreateReadParams(repo),
Revision: rev,
IgnoreWhitespace: ignoreWhitespace,
}, w)
}
@ -98,6 +102,7 @@ func (c *Controller) DiffStats(
session *auth.Session,
repoRef string,
path string,
ignoreWhitespace bool,
) (types.DiffStats, error) {
repo, err := c.repoFinder.FindByRef(ctx, repoRef)
if err != nil {
@ -118,6 +123,7 @@ func (c *Controller) DiffStats(
BaseRef: info.BaseRef,
HeadRef: info.HeadRef,
MergeBase: info.MergeBase,
IgnoreWhitespace: ignoreWhitespace,
})
if err != nil {
return types.DiffStats{}, err
@ -132,6 +138,7 @@ func (c *Controller) Diff(
repoRef string,
path string,
includePatch bool,
ignoreWhitespace bool,
files ...gittypes.FileDiffRequest,
) (types.Stream[*git.FileDiff], error) {
repo, err := c.repoFinder.FindByRef(ctx, repoRef)
@ -154,6 +161,7 @@ func (c *Controller) Diff(
HeadRef: info.HeadRef,
MergeBase: info.MergeBase,
IncludePatch: includePatch,
IgnoreWhitespace: ignoreWhitespace,
}, files...))
return reader, nil

View File

@ -70,8 +70,26 @@ func HandleDiff(pullreqCtrl *pullreq.Controller) http.HandlerFunc {
return
}
_, includePatch := request.QueryParam(r, "include_patch")
stream, err := pullreqCtrl.Diff(ctx, session, repoRef, pullreqNumber, setSHAs, includePatch, files...)
includePatch, err := request.QueryParamAsBoolOrDefault(r, "include_patch", false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
ignoreWhitespace, err := request.QueryParamAsBoolOrDefault(r, request.QueryParamIgnoreWhitespace, false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
stream, err := pullreqCtrl.Diff(
ctx,
session,
repoRef,
pullreqNumber,
setSHAs,
includePatch,
ignoreWhitespace,
files...,
)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return

View File

@ -53,16 +53,41 @@ func HandleDiff(repoCtrl *repo.Controller) http.HandlerFunc {
files = request.GetFileDiffFromQuery(r)
}
ignoreWhitespace, err := request.QueryParamAsBoolOrDefault(r, request.QueryParamIgnoreWhitespace, false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
if strings.HasPrefix(r.Header.Get("Accept"), "text/plain") {
err := repoCtrl.RawDiff(ctx, w, session, repoRef, path, files...)
err := repoCtrl.RawDiff(
ctx,
w,
session,
repoRef,
path,
ignoreWhitespace,
files...,
)
if err != nil {
http.Error(w, err.Error(), http.StatusOK)
}
return
}
_, includePatch := request.QueryParam(r, "include_patch")
stream, err := repoCtrl.Diff(ctx, session, repoRef, path, includePatch, files...)
includePatch, err := request.QueryParamAsBoolOrDefault(r, request.QueryParamIncludePatch, false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
stream, err := repoCtrl.Diff(
ctx,
session,
repoRef,
path,
includePatch,
ignoreWhitespace,
files...,
)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
@ -89,7 +114,19 @@ func HandleCommitDiff(repoCtrl *repo.Controller) http.HandlerFunc {
return
}
err = repoCtrl.CommitDiff(ctx, session, repoRef, commitSHA, w)
ignoreWhitespace, err := request.QueryParamAsBoolOrDefault(r, request.QueryParamIgnoreWhitespace, false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
err = repoCtrl.CommitDiff(
ctx,
session,
repoRef,
commitSHA,
ignoreWhitespace,
w,
)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
@ -110,7 +147,18 @@ func HandleDiffStats(repoCtrl *repo.Controller) http.HandlerFunc {
path := request.GetOptionalRemainderFromPath(r)
output, err := repoCtrl.DiffStats(ctx, session, repoRef, path)
ignoreWhitespace, err := request.QueryParamAsBoolOrDefault(r, request.QueryParamIgnoreWhitespace, false)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
output, err := repoCtrl.DiffStats(
ctx,
session,
repoRef,
path,
ignoreWhitespace,
)
if uErr := gittypes.AsUnrelatedHistoriesError(err); uErr != nil {
render.JSON(w, http.StatusOK, &usererror.Error{
Message: uErr.Error(),

View File

@ -137,11 +137,13 @@ type fileViewDeletePullReqRequest struct {
type getRawPRDiffRequest struct {
pullReqRequest
Path []string `query:"path" description:"provide path for diff operation"`
IgnoreWhitespace bool `query:"ignore_whitespace" required:"false" default:"false"`
}
type postRawPRDiffRequest struct {
pullReqRequest
gittypes.FileDiffRequests
IgnoreWhitespace bool `query:"ignore_whitespace" required:"false" default:"false"`
}
type getPullReqChecksRequest struct {

View File

@ -121,6 +121,11 @@ type GetCommitRequest struct {
CommitSHA string `path:"commit_sha"`
}
type GetCommitDiffRequest struct {
GetCommitRequest
IgnoreWhitespace bool `query:"ignore_whitespace" required:"false" default:"false"`
}
type calculateCommitDivergenceRequest struct {
repoRequest
repo.GetCommitDivergencesInput
@ -162,12 +167,14 @@ type getRawDiffRequest struct {
repoRequest
Range string `path:"range" example:"main..dev"`
Path []string `query:"path" description:"provide path for diff operation"`
IgnoreWhitespace bool `query:"ignore_whitespace" required:"false" default:"false"`
}
type postRawDiffRequest struct {
repoRequest
gittypes.FileDiffRequests
Range string `path:"range" example:"main..dev"`
IgnoreWhitespace bool `query:"ignore_whitespace" required:"false" default:"false"`
}
type codeOwnersValidate struct {
@ -1104,7 +1111,7 @@ func repoOperations(reflector *openapi3.Reflector) {
opCommitDiff := openapi3.Operation{}
opCommitDiff.WithTags("repository")
opCommitDiff.WithMapOfAnything(map[string]interface{}{"operationId": "getCommitDiff"})
_ = reflector.SetRequest(&opCommitDiff, new(GetCommitRequest), http.MethodGet)
_ = reflector.SetRequest(&opCommitDiff, new(GetCommitDiffRequest), http.MethodGet)
_ = reflector.SetStringResponse(&opCommitDiff, http.StatusOK, "text/plain")
_ = reflector.SetJSONResponse(&opCommitDiff, new(usererror.Error), http.StatusInternalServerError)
_ = reflector.SetJSONResponse(&opCommitDiff, new(usererror.Error), http.StatusUnauthorized)

20
app/api/request/diff.go Normal file
View File

@ -0,0 +1,20 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package request
const (
QueryParamIgnoreWhitespace = "ignore_whitespace"
QueryParamIncludePatch = "include_patch"
)

View File

@ -164,6 +164,7 @@ func (g *Git) RawDiff(
baseRef string,
headRef string,
mergeBase bool,
ignoreWhitespace bool,
alternates []string,
files ...FileDiffRequest,
) error {
@ -190,6 +191,11 @@ func (g *Git) RawDiff(
cmd.Add(command.WithFlag("--merge-base"))
}
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
perFileDiffRequired := false
paths := make([]string, 0, len(files))
if len(files) > 0 {
@ -266,6 +272,7 @@ func (g *Git) CommitDiff(
ctx context.Context,
repoPath string,
rev string,
ignoreWhitespace bool,
w io.Writer,
) error {
if repoPath == "" {
@ -281,6 +288,11 @@ func (g *Git) CommitDiff(
command.WithArg(rev),
)
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
if err := cmd.Run(ctx,
command.WithDir(repoPath),
command.WithStdout(w),
@ -296,6 +308,7 @@ func (g *Git) DiffShortStat(
baseRef string,
headRef string,
useMergeBase bool,
ignoreWhitespace bool,
) (DiffShortStat, error) {
if repoPath == "" {
return DiffShortStat{}, ErrRepositoryPathEmpty
@ -309,7 +322,7 @@ func (g *Git) DiffShortStat(
if len(baseRef) == 0 || baseRef == types.NilSHA {
shortstatArgs = []string{sha.EmptyTree.String(), headRef}
}
stat, err := GetDiffShortStat(ctx, repoPath, shortstatArgs...)
stat, err := GetDiffShortStat(ctx, repoPath, ignoreWhitespace, shortstatArgs...)
if err != nil {
return DiffShortStat{}, processGitErrorf(err, "failed to get diff short stat between %s and %s",
baseRef, headRef)
@ -348,6 +361,7 @@ func (g *Git) GetDiffHunkHeaders(
command.WithArg(sourceRef),
command.WithArg(targetRef),
)
err = cmd.Run(ctx,
command.WithDir(repoPath),
command.WithStdout(pipeWrite),
@ -380,6 +394,7 @@ func (g *Git) DiffCut(
targetRef string,
sourceRef string,
path string,
ignoreWhitespace bool,
params parser.DiffCutParams,
) (parser.HunkHeader, parser.Hunk, error) {
if repoPath == "" {
@ -404,6 +419,10 @@ func (g *Git) DiffCut(
command.WithFlag("--find-renames"),
command.WithArg(targetRef),
command.WithArg(sourceRef))
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
err = cmd.Run(ctx, command.WithDir(repoPath), command.WithStdout(pipeWrite))
}()
@ -468,7 +487,7 @@ func (g *Git) DiffCut(
// For modified and renamed compare file blob SHAs directly.
oldSHA = entry.OldBlobSHA
newSHA = entry.NewBlobSHA
hunkHeader, hunk, err = g.diffCutFromHunk(ctx, repoPath, oldSHA, newSHA, params)
hunkHeader, hunk, err = g.diffCutFromHunk(ctx, repoPath, oldSHA, newSHA, ignoreWhitespace, params)
case parser.DiffStatusAdded, parser.DiffStatusDeleted, parser.DiffStatusType:
// for added and deleted files read the file content directly
if params.LineStartNew {
@ -501,6 +520,7 @@ func (g *Git) diffCutFromHunk(
repoPath string,
oldSHA string,
newSHA string,
ignoreWhitespace bool,
params parser.DiffCutParams,
) (parser.HunkHeader, parser.Hunk, error) {
pipeRead, pipeWrite := io.Pipe()
@ -521,6 +541,11 @@ func (g *Git) diffCutFromHunk(
command.WithArg(oldSHA),
command.WithArg(newSHA))
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
err = cmd.Run(ctx,
command.WithDir(repoPath),
command.WithStdout(pipeWrite),
@ -614,6 +639,7 @@ func (g *Git) DiffFileName(ctx context.Context,
baseRef string,
headRef string,
mergeBase bool,
ignoreWhitespace bool,
) ([]string, error) {
cmd := command.New("diff", command.WithFlag("--name-only"))
if mergeBase {
@ -621,6 +647,11 @@ func (g *Git) DiffFileName(ctx context.Context,
}
cmd.Add(command.WithArg(baseRef, headRef))
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
stdout := &bytes.Buffer{}
err := cmd.Run(ctx,
command.WithDir(repoPath),
@ -637,6 +668,7 @@ func (g *Git) DiffFileName(ctx context.Context,
func GetDiffShortStat(
ctx context.Context,
repoPath string,
ignoreWhitespace bool,
args ...string,
) (DiffShortStat, error) {
// Now if we call:
@ -649,6 +681,11 @@ func GetDiffShortStat(
command.WithArg(args...),
)
if ignoreWhitespace {
// Ignore whitespace when comparing lines.
cmd.Add(command.WithFlag("-w"))
}
stdout := &bytes.Buffer{}
if err := cmd.Run(ctx,
command.WithDir(repoPath),

View File

@ -40,6 +40,7 @@ func CommitMessage(subject, body string) string {
type GetCommitParams struct {
ReadParams
Revision string
IgnoreWhitespace bool
}
type Commit struct {

View File

@ -37,6 +37,7 @@ type DiffParams struct {
HeadRef string
MergeBase bool
IncludePatch bool
IgnoreWhitespace bool
}
func (p DiffParams) Validate() error {
@ -72,6 +73,7 @@ func (s *Service) rawDiff(ctx context.Context, w io.Writer, params *DiffParams,
params.BaseRef,
params.HeadRef,
params.MergeBase,
params.IgnoreWhitespace,
params.AlternateObjectDirs,
files...,
)
@ -83,7 +85,13 @@ func (s *Service) rawDiff(ctx context.Context, w io.Writer, params *DiffParams,
func (s *Service) CommitDiff(ctx context.Context, params *GetCommitParams, out io.Writer) error {
repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID)
err := s.git.CommitDiff(ctx, repoPath, params.Revision, out)
err := s.git.CommitDiff(
ctx,
repoPath,
params.Revision,
params.IgnoreWhitespace,
out,
)
if err != nil {
return err
}
@ -107,6 +115,7 @@ func (s *Service) DiffShortStat(ctx context.Context, params *DiffParams) (DiffSh
params.BaseRef,
params.HeadRef,
params.MergeBase,
params.IgnoreWhitespace,
)
if err != nil {
return DiffShortStatOutput{}, err
@ -168,6 +177,7 @@ func (s *Service) DiffStats(ctx context.Context, params *DiffParams) (DiffStatsO
BaseRef: params.BaseRef,
HeadRef: params.HeadRef,
MergeBase: true, // must be true, because commitDivergences use triple dot notation
IgnoreWhitespace: params.IgnoreWhitespace,
})
if err != nil {
return err
@ -222,7 +232,12 @@ func (s *Service) GetDiffHunkHeaders(
repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID)
hunkHeaders, err := s.git.GetDiffHunkHeaders(ctx, repoPath, params.TargetCommitSHA, params.SourceCommitSHA)
hunkHeaders, err := s.git.GetDiffHunkHeaders(
ctx,
repoPath,
params.TargetCommitSHA,
params.SourceCommitSHA,
)
if err != nil {
return GetDiffHunkHeadersOutput{}, err
}
@ -261,6 +276,7 @@ type DiffCutParams struct {
LineEnd int
LineEndNew bool
LineLimit int
IgnoreWhitespace bool
}
// DiffCut extracts diff snippet from a git diff hunk.
@ -277,11 +293,13 @@ func (s *Service) DiffCut(ctx context.Context, params *DiffCutParams) (DiffCutOu
return DiffCutOutput{}, fmt.Errorf("failed to find merge base: %w", err)
}
header, linesHunk, err := s.git.DiffCut(ctx,
header, linesHunk, err := s.git.DiffCut(
ctx,
repoPath,
params.TargetCommitSHA,
params.SourceCommitSHA,
params.Path,
params.IgnoreWhitespace,
parser.DiffCutParams{
LineStart: params.LineStart,
LineStartNew: params.LineStartNew,
@ -290,7 +308,8 @@ func (s *Service) DiffCut(ctx context.Context, params *DiffCutParams) (DiffCutOu
BeforeLines: 2,
AfterLines: 2,
LineLimit: params.LineLimit,
})
},
)
if err != nil {
return DiffCutOutput{}, fmt.Errorf("DiffCut: failed to get diff hunk: %w", err)
}
@ -425,6 +444,7 @@ func (s *Service) DiffFileNames(ctx context.Context, params *DiffParams) (DiffFi
params.BaseRef,
params.HeadRef,
params.MergeBase,
params.IgnoreWhitespace,
)
if err != nil {
return DiffFileNamesOutput{}, fmt.Errorf("failed to get diff file data between '%s' and '%s': %w",

View File

@ -209,7 +209,14 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
// find short stat and number of commits
shortStat, err := s.git.DiffShortStat(ctx, repoPath, baseCommitSHA.String(), headCommitSHA.String(), true)
shortStat, err := s.git.DiffShortStat(
ctx,
repoPath,
baseCommitSHA.String(),
headCommitSHA.String(),
true,
false,
)
if err != nil {
return MergeOutput{}, errors.Internal(err,
"failed to find short stat between %s and %s", baseCommitSHA, headCommitSHA)