Skip to content

Commit ecfe792

Browse files
authored
Merge pull request #168 from githubnext/copilot/prettify-markdown-rendering
2 parents c19de04 + 125ecce commit ecfe792

2 files changed

Lines changed: 85 additions & 19 deletions

File tree

internal/logger/rpc_logger.go

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -140,36 +140,46 @@ func formatRPCMessage(info *RPCMessageInfo) string {
140140
}
141141

142142
// formatJSONWithoutFields formats JSON by removing specified fields and indenting with 2 spaces
143-
// Returns the formatted string and a boolean indicating if the JSON was valid
144-
func formatJSONWithoutFields(jsonStr string, fieldsToRemove []string) (string, bool) {
143+
// Returns the formatted string, a boolean indicating if the JSON was valid, and a boolean indicating if empty
144+
func formatJSONWithoutFields(jsonStr string, fieldsToRemove []string) (string, bool, bool) {
145145
var data map[string]interface{}
146146
if err := json.Unmarshal([]byte(jsonStr), &data); err != nil {
147147
// If not valid JSON, return as-is with false
148-
return jsonStr, false
148+
return jsonStr, false, false
149149
}
150150

151151
// Remove specified fields
152152
for _, field := range fieldsToRemove {
153153
delete(data, field)
154154
}
155155

156-
// Re-marshal with 2-space indentation
156+
// Check if only "params": null remains (or equivalent empty state)
157+
isEmpty := isEffectivelyEmpty(data)
158+
159+
// Re-marshal with 2-space indentation (pretty print)
157160
formatted, err := json.MarshalIndent(data, "", " ")
158161
if err != nil {
159-
return jsonStr, false
162+
return jsonStr, false, false
163+
}
164+
165+
return string(formatted), true, isEmpty
166+
}
167+
168+
// isEffectivelyEmpty checks if the data is effectively empty (only contains params: null)
169+
func isEffectivelyEmpty(data map[string]interface{}) bool {
170+
// If empty, it's empty
171+
if len(data) == 0 {
172+
return true
160173
}
161174

162-
// Compress JSON: remove newline after opening brace and before closing brace
163-
result := string(formatted)
164-
// Remove newline after opening brace
165-
result = strings.Replace(result, "{\n", "{ ", 1)
166-
// Remove newline before closing brace
167-
lastNewline := strings.LastIndex(result, "\n}")
168-
if lastNewline != -1 {
169-
result = result[:lastNewline] + " }"
175+
// If only one field and it's "params" with null value, it's empty
176+
if len(data) == 1 {
177+
if params, ok := data["params"]; ok && params == nil {
178+
return true
179+
}
170180
}
171181

172-
return result, true
182+
return false
173183
}
174184

175185
// formatRPCMessageMarkdown formats an RPC message for markdown logging
@@ -196,11 +206,14 @@ func formatRPCMessageMarkdown(info *RPCMessageInfo) string {
196206
// Add formatted payload in code block
197207
if info.Payload != "" {
198208
// Remove jsonrpc and method fields, then format
199-
formatted, isValidJSON := formatJSONWithoutFields(info.Payload, []string{"jsonrpc", "method"})
209+
formatted, isValidJSON, isEmpty := formatJSONWithoutFields(info.Payload, []string{"jsonrpc", "method"})
200210
if isValidJSON {
201-
// Valid JSON: use code block for better readability (compact formatting)
202-
// Empty line before ~~~ per markdown convention
203-
message += fmt.Sprintf(" \n\n~~~\n%s\n~~~", formatted)
211+
// Don't show JSON block if it's effectively empty (only params: null)
212+
if !isEmpty {
213+
// Valid JSON: use code block for better readability (pretty printed)
214+
// Empty line before ~~~ per markdown convention
215+
message += fmt.Sprintf(" \n\n~~~\n%s\n~~~", formatted)
216+
}
204217
} else {
205218
// Invalid JSON: use inline backticks to avoid malformed markdown
206219
message += fmt.Sprintf(" `%s`", formatted)

internal/logger/rpc_logger_test.go

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,32 @@ func TestFormatRPCMessageMarkdown(t *testing.T) {
234234
want: []string{"**github**→`tools/call`", "`{invalid json syntax}`"},
235235
notWant: []string{"~~~"}, // Should NOT use code blocks for invalid JSON
236236
},
237+
{
238+
name: "request with only params null after field removal",
239+
info: &RPCMessageInfo{
240+
Direction: RPCDirectionOutbound,
241+
MessageType: RPCMessageRequest,
242+
ServerID: "github",
243+
Method: "tools/list",
244+
PayloadSize: 50,
245+
Payload: `{"jsonrpc":"2.0","method":"tools/list","params":null}`,
246+
},
247+
want: []string{"**github**→`tools/list`"},
248+
notWant: []string{"~~~", `"params"`}, // Should NOT show JSON block when only params: null
249+
},
250+
{
251+
name: "request with empty object after field removal",
252+
info: &RPCMessageInfo{
253+
Direction: RPCDirectionOutbound,
254+
MessageType: RPCMessageRequest,
255+
ServerID: "github",
256+
Method: "tools/list",
257+
PayloadSize: 50,
258+
Payload: `{"jsonrpc":"2.0","method":"tools/list"}`,
259+
},
260+
want: []string{"**github**→`tools/list`"},
261+
notWant: []string{"~~~"}, // Should NOT show JSON block when empty
262+
},
237263
}
238264

239265
for _, tt := range tests {
@@ -263,6 +289,7 @@ func TestFormatJSONWithoutFields(t *testing.T) {
263289
wantContains []string
264290
wantNotContain []string
265291
wantValid bool
292+
wantEmpty bool
266293
}{
267294
{
268295
name: "remove jsonrpc and method",
@@ -271,6 +298,7 @@ func TestFormatJSONWithoutFields(t *testing.T) {
271298
wantContains: []string{`"params"`, `"arg"`, `"value"`, `"id"`},
272299
wantNotContain: []string{`"jsonrpc"`, `"method"`},
273300
wantValid: true,
301+
wantEmpty: false,
274302
},
275303
{
276304
name: "indent with 2 spaces",
@@ -279,6 +307,7 @@ func TestFormatJSONWithoutFields(t *testing.T) {
279307
wantContains: []string{" \"a\"", " \"c\"", " \"d\""},
280308
wantNotContain: []string{},
281309
wantValid: true,
310+
wantEmpty: false,
282311
},
283312
{
284313
name: "invalid JSON returns as-is with false",
@@ -287,6 +316,7 @@ func TestFormatJSONWithoutFields(t *testing.T) {
287316
wantContains: []string{`{invalid json}`},
288317
wantNotContain: []string{},
289318
wantValid: false,
319+
wantEmpty: false,
290320
},
291321
{
292322
name: "empty object",
@@ -295,17 +325,40 @@ func TestFormatJSONWithoutFields(t *testing.T) {
295325
wantContains: []string{`{}`},
296326
wantNotContain: []string{},
297327
wantValid: true,
328+
wantEmpty: true,
329+
},
330+
{
331+
name: "only params null after removal",
332+
input: `{"jsonrpc":"2.0","method":"tools/list","params":null}`,
333+
fieldsToRemove: []string{"jsonrpc", "method"},
334+
wantContains: []string{`"params"`, `null`},
335+
wantNotContain: []string{},
336+
wantValid: true,
337+
wantEmpty: true,
338+
},
339+
{
340+
name: "params with value is not empty",
341+
input: `{"jsonrpc":"2.0","method":"tools/list","params":{"key":"value"}}`,
342+
fieldsToRemove: []string{"jsonrpc", "method"},
343+
wantContains: []string{`"params"`},
344+
wantNotContain: []string{},
345+
wantValid: true,
346+
wantEmpty: false,
298347
},
299348
}
300349

301350
for _, tt := range tests {
302351
t.Run(tt.name, func(t *testing.T) {
303-
result, isValid := formatJSONWithoutFields(tt.input, tt.fieldsToRemove)
352+
result, isValid, isEmpty := formatJSONWithoutFields(tt.input, tt.fieldsToRemove)
304353

305354
if isValid != tt.wantValid {
306355
t.Errorf("Expected isValid=%v, got %v", tt.wantValid, isValid)
307356
}
308357

358+
if isEmpty != tt.wantEmpty {
359+
t.Errorf("Expected isEmpty=%v, got %v", tt.wantEmpty, isEmpty)
360+
}
361+
309362
for _, want := range tt.wantContains {
310363
if !strings.Contains(result, want) {
311364
t.Errorf("Expected result to contain %q, got:\n%s", want, result)

0 commit comments

Comments
 (0)