Debug: []byte/[]uint8/[]rune should be displayed as strings

[willfaught]

And then provide a way to drill down to see individual indexes and their elements.

For example:

---

• VSCode Version: Code 1.18.1 (929bacba01ef658b873545e26034d1a8067445e9, 2017-11-16T18:23:26.125Z)
• OS Version: Darwin x64 17.2.0
• Extensions:

Extension	Author (truncated)	Version
vscode-docker	Pet	0.0.22
sort-lines	Tyr	1.3.0
githistory	don	0.2.3
markdown-table-formatter	jos	0.2.3
Go	luk	0.6.69
prettify-json	moh	0.0.3
azure-account	ms-	0.2.2
rewrap	stk	1.5.3
gitblame	wad	2.2.0
vscode-proto3	zxh	0.1.2

[ramya-rao-a]

Currently, we show the variables as returned by dlv. See #1358 (comment)

If dlv were to return the string for the []rune or []byte, then it would be straight forward for us to show it.

If we are to show the string form of the runes/bytes, then we need to first

• convert the rune/byte to string
• Add an extra variable with the same name as the []rune with say " as string" appended to it

These changes will have to go to either the scopesRequest or the variablesRequest in https://github.com/Microsoft/vscode-go/blob/0.6.69/src/debugAdapter/goDebug.ts. I am not sure.

One catch is that I dont know how the watch feature would work with this new variable. And of course the debug console wont recognise it.

[willfaught]

Couldn't we show the string instead of the array syntax to avoid needing a new variable?

[ramya-rao-a]

Is there a scenario where one would want to look at the runes/bytes values?

[willfaught]

I suppose if you’re looking at a non-Unicode data source, like an image, at least for bytes. Probably not a need for runes, since they’d display the same as in strings.

…

On Dec 4, 2017, at 11:36 PM, Ramya Rao ***@***.***> wrote: Is there a scenario where one would want to look at the runes/bytes values? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[lggomez]

In my team we're really interested in having this feature since we tend to work a lot with responses and encoded strings, so It'd be a great addition to the extension

We could prepend it to the array values (sort of like the behavior that triggered the changes in #1384)

I made a proof of concept here: master...lggomez:values_1385

It seems to be working fine, is watch-friendly and also works fine with the variable lens (although I couldn't get it in the screenshot) but more test cases are probably needed:

This currently has 2 caveats:

• For some reason, the kinds returned by delve for the example slices are GoReflectKind.Uint
• Since the conversion from value to string is performed locally by the extension, I don't know if we have the guarantee that the resulting string values will match the ones produced by go (at least for runes)

This is the example code I'm using

package main

import "fmt"

type foo struct {
	a []uint8
	b []byte
	c []byte
	d int
}

func helloworld() string {
	asd := foo{a: []uint8("Hello World!!"), b: []byte("Hello World!!"), c: []byte(""), d: 42}
	return string(asd.a)
}

func main() {
	fmt.Println(helloworld())
}

[ramya-rao-a]

@lggomez

the kinds returned by delve for the example slices are GoReflectKind.Uint

Thats because strings are built from bytes so indexing them yields bytes. See
https://blog.golang.org/strings

Since the conversion from value to string is performed locally by the extension, I don't know if we have the guarantee that the resulting string values will match the ones produced by go

Agreed, this should be ok

I checked your code. It doest work for runes though. Try

r := []rune("hello")
fmt.Println(r[0])

[lggomez]

I included runes. These kinds should include most cases:
childKind === GoReflectKind.Uint || childKind === GoReflectKind.Uint8 || childKind === GoReflectKind.Int || childKind === GoReflectKind.Int32

Also added some exception handling and refactored the logic into a separate method

Does the debugger handle the value length when loading it for the lens/watch or should we set a max length when converting the slice?

If this feature looks ok to push forward, should it be able to be enabled/disabled via a new setting property?

[ramya-rao-a]

childKind === GoReflectKind.Uint || childKind === GoReflectKind.Uint8 || childKind === GoReflectKind.Int || childKind === GoReflectKind.Int32

What about the scenario where there would be an array/slice of kind int/int32 that is not meant to be string? Do you know of any documentation that can help us understand these types and their intended usage better?

Does the debugger handle the value length when loading it for the lens/watch or should we set a max length when converting the slice?

The debugger shows whats sent over by delve. Delve already truncates the array. See https://github.com/derekparker/delve/blob/62fe792bfdf6599ad06c084096efd96a01246ab8/service/api/types.go#L192-L206

should it be able to be enabled/disabled via a new setting property

Let's not add any setting and watch for user reaction instead.

[lggomez]

What about the scenario where there would be an array/slice of kind int/int32 that is not meant to be string? Do you know of any documentation that can help us understand these types and their intended usage better?

That's very tricky, I don't think we have any runtime information exposed by the debugger which can help us to infer these cases so it's either adding this display value or not, hence my suggestion of making it optional

Of course, I may be overlooking something useful within this context, so any other suggestions are more than welcome

[lggomez]

The debugger shows whats sent over by delve. Delve already truncates the array. See https://github.com/derekparker/delve/blob/62fe792bfdf6599ad06c084096efd96a01246ab8/service/api/types.go#L192-L206

About this, I found the following:
https://github.com/derekparker/delve/issues/609#issuecomment-239161459
https://github.com/derekparker/delve/issues/659#issuecomment-256945278

It seems delve pages string array values, so changing the size param or getting the next n elements could be an option. I'm not quite familiar with the code yet so I don't know the feasibility of including this feature. This should go into a separate issue though

[ramya-rao-a]

Can you check how gdlv deals with this?

About the array length, yes that can be a separate issue

[ramya-rao-a]

@jhendrixMSFT, @vladbarosan , @quoctruong , @iantalarico and @pongad Any thoughts on this?

[quoctruong]

@lggomez @ramya-rao-a I don't know how useful this feature would be. I don't think any other debuggers for VSCode has this feature? Also, I think user can also just make and evaluate the string in the Watch panel directly if they want to view it as a string?

[superhawk610]

@quoctruong your solution is a useful workaround. What's the best way (in your opinion) to make/evaluate a variable for debugging without having Go complain about an unused variable declaration?

For example, I have a tokenizer that switches on a rune and I want to see the string representation of the rune when debugging:

r, _, err := f.stream.ReadRune()

s := string([]rune{r}) // <- this prevents compilation
_ = string([]rune{r})  // <- this doesn't show up in the debugger
fmt.Println(s)         // <- this is what I'm currently doing so `s` isn't unused,
                       //    but there's got to be a better way, right?

switch r { /* ... */ }

[wtask]

If VSCode will have an option to display values as is or as string without to use Watch it will be very progressive. We don't know what is the context of underlying data, any on the fly converters will be usefull.

[pongad]

@superhawk610 I think you can do something like

s := string(r) // You can convert rune directly to string!
_ = s

so that s remains used. I agree it's still annoying though.

[stamblerre]

I have filed golang/vscode-go#162 in our new repository (#3247) so let's continue this discussion there. Thanks all!