graphqlbackend: Implement diff and reproducedDiff #47310
Conversation
In this commit we make the following changes to the SiteConfigurationChange API: - Add a new field reproducedDiff to indicate if Sourcegraph was able to generate the diff or not - Make the Diff field nullable - Implement the Diff field which was previously hardcoded to return an empty strings
indradhanush
added
feature
| // One line wrapper to be able to use in tests as well. | ||
| func marshalSiteConfigurationChangeID(id int32) graphql.ID { | ||
| return relay.MarshalID(siteConfigurationChangeKind, &id) | ||
| } | ||
|
|
There was a problem hiding this comment.
No change. Just moved it out of the way of the API methods and moved it all the way to the end of the file.
|
|
||
| // We're not diffing a file, so set an empty string for the URI argument. | ||
| edits := myers.ComputeEdits("", prevRedactedContents, r.siteConfig.RedactedContents) | ||
| diff := fmt.Sprint(gotextdiff.ToUnified(prettyID(prevID), prettyID(r.siteConfig.ID), prevRedactedContents, edits)) |
There was a problem hiding this comment.
Does this require a call to fmt.Sprint or would string() be enough?
There was a problem hiding this comment.
fmt.Sprint is required. The return type of gotextdiff.ToUnified does not have a String method but has a Format method: https://sourcegraph.com/github.com/hexops/gotextdiff/-/blob/unified.go?L165&subtree=true
There was a problem hiding this comment.
Got it! (Might be a chance to open a tiny PR on hexops/gotextdiff and add the String() method that does exactly this: fmt.Sprint(u) 🙂 )
There was a problem hiding this comment.
Great suggestion! PR filed: hexops/gotextdiff#3
Co-authored-by: Thorsten Ball <[email protected]>
Co-authored-by: Thorsten Ball <[email protected]>
| "author": null | ||
| "author": null, | ||
| "reproducedDiff": true, | ||
| "diff": %[6]q |
There was a problem hiding this comment.
I think the indentation is off for the new lines. Missing some <tab>s I think
There was a problem hiding this comment.
Huh. I think this is what happened. 😂
edit: this image, turns out images are not embedded in comments
| } | ||
| }, | ||
| "reproducedDiff": true, | ||
| "diff": %[4]q |
There was a problem hiding this comment.
Same here, whitespace is off.
| "author": null | ||
| "author": null, | ||
| "reproducedDiff": true, | ||
| "diff": %[4]q |
Fixes #46031.
In this commit we make the following changes to the
SiteConfigurationChange API:
generate the diff or not
empty strings
Example API output:
👉🏽 Try it out yourself:
git fetch && git checkout ig/pretty-diff-apisg startTest plan