Skip to content
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

add: json diff viewer #1

Merged
merged 13 commits into from
Oct 15, 2024
Merged

add: json diff viewer #1

merged 13 commits into from
Oct 15, 2024

Conversation

shivamsouravjha
Copy link
Collaborator

No description provided.

Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
jsondiff.go Outdated
// ColorizedResponse holds the colorized differences between the expected and actual JSON responses.
// ExpectedResponse: The colorized string representing the differences in the expected JSON response.
// ActualResponse: The colorized string representing the differences in the actual JSON response.
type ColorisedResponse struct {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just call it diff?

jsondiff.go Outdated
// ExpectedResponse: The colorized string representing the differences in the expected JSON response.
// ActualResponse: The colorized string representing the differences in the actual JSON response.
type ColorisedResponse struct {
ExpectedResponse string
Copy link
Member

Choose a reason for hiding this comment

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

these could be expected and actual?

jsondiff.go Outdated
// json2: The second JSON object to compare.
// noise: A map containing fields to ignore during the comparison.
// Returns a ColorizedResponse containing the colorized differences for the expected and actual JSON responses.
func ColorJSONDiff(json1 []byte, json2 []byte, noise map[string][]string) (ColorisedResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

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

CompareJSON?

jsondiff.go Outdated
// expect: The JSON string containing the expected values.
// actual: The JSON string containing the actual values.
// Returns a ColorizedResponse containing the colorized differences for the expected and actual JSON responses.
func ColorDiff(expect, actual string) ColorisedResponse {
Copy link
Member

Choose a reason for hiding this comment

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

Compare?

jsondiff.go Outdated
// expect: The map containing the expected header values.
// actual: The map containing the actual header values.
// Returns a ColorizedResponse containing the colorized differences for the expected and actual headers.
func ColorHeaderDiff(expect, actual map[string]string) ColorisedResponse {
Copy link
Member

Choose a reason for hiding this comment

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

CompareHeaders?

Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
jsondiff.go Outdated
End int // End is the ending index of the range.
}

// ColorizedResponse holds the colorized differences between the expected and actual JSON responses.
Copy link
Member

Choose a reason for hiding this comment

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

Change the comment, since there is not struct named as ColorizedReponse

}

// isControlCharacter checks if a character is a control character.
func isControlCharacter(char rune) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Please explain it more. This doesn't explain what a control character is.

jsondiff.go Outdated
}

// maxLineLength is the maximum length of a line before it is wrapped.
const max_line_length = 50
Copy link
Member

Choose a reason for hiding this comment

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

Don't use snakeCase

@gouravkrosx
Copy link
Member

@shivamsouravjha Add a pipeline to trigger the unit tests.

Signed-off-by: shivamsouravjha <[email protected]>
Signed-off-by: shivamsouravjha <[email protected]>
if !reflect.DeepEqual(val1, val2) {
// Marshal values to pretty-printed JSON strings
val1Str, err := json.MarshalIndent(val1, "", " ")
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

why are you not printing the error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

considering this to be a sdk I thought of not adding print statements.

}
val2Str, err := json.MarshalIndent(val2, "", " ")
if err != nil {
return
Copy link
Member

Choose a reason for hiding this comment

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

Here as well.

nextLine := lines[i+1]

// Process lines that start with a '-' indicating expected differences.
if len(line) > 0 && line[0] == '-' && i != len(lines)-1 {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think that the condition len(line)>0 can happen anytime?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes ,I've explicitly appended empty string in the lines in

lines = insertEmptyLines(lines) // Insert empty lines between consecutive elements with the same symbol.

So that two ++ signs don't come after each other

Copy link
Member

@gouravkrosx gouravkrosx left a comment

Choose a reason for hiding this comment

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

Please address the comments, rest LGTM!

Signed-off-by: shivamsouravjha <[email protected]>
@slayerjain slayerjain merged commit b98580f into main Oct 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants