vscode-go icon indicating copy to clipboard operation
vscode-go copied to clipboard

debug: variable watcher escapes json inside []byte

Open gudvinr opened this issue 4 years ago • 5 comments

What version of Go, VS Code & VS Code Go extension are you using?

Version Information
  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.17.1 linux/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.7.2
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.60.2
  • Check your installed extensions to get the version of the VS Code Go extension
    • v0.28.1
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
    • gopkgs: ~/tools/go/path/bin/gopkgs: go1.17.1 go-outline: ~/tools/go/path/bin/go-outline: go1.17.1 gotests: ~/tools/go/path/bin/gotests: go1.17.1 gomodifytags: ~/tools/go/path/bin/gomodifytags: go1.17.1 impl: ~/tools/go/path/bin/impl: go1.17.1 goplay: not installed dlv: ~/tools/go/path/bin/dlv: go1.17.1 dlv-dap: ~/tools/go/path/bin/dlv-dap: go1.17.1

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file. Share all the settings with the go. or ["go"] or gopls prefixes.

    "go.toolsManagement.autoUpdate": true,
    "go.vetOnSave": "workspace",
    "go.lintOnSave": "workspace",
    "go.toolsGopath": "~/tools/go/path",
    "gopls": {
        "build.experimentalWorkspaceModule": false,
        "ui.documentation.linksInHover": false,
        "usePlaceholders": false,
        "analyses": {
            "composites": false,
            "shadow": true,
            "ST1000": false,
            "ST1003": false
        },
        "ui.diagnostic.staticcheck": true
    }

Describe the bug

Variable watcher in debug view applies JSON escaping rules to string() if underlying type of variable []byte.

As you can see on screenshot, byte view shows 123 34 116 34 58 110 117 108 108 44 which is the representation of {"t":null, in ASCII/UTF-8.
But string() showing {\"t\":null, and it is incorrect representation of characters in input byte array.

Steps to reproduce the behavior:

  1. Place breakpoint where you can see []byte variable filled with valid JSON
  2. Go to Debug View panel
  3. Start debug session
  4. Wait for breakpoint to trigger

Screenshots or recordings

image

gudvinr avatar Oct 01 '21 00:10 gudvinr

Thanks for the report. From the screenshot, I see: The string() shows Go syntax string representation "{\"t\":null, ..." which is correct strictly speaking IMO.

But I see that some users may be annoyed to see the escape character. @suzmue @polinasok is it possible to send unescaped string (not go syntax representation) for this display context? i.e., {"t": ...

I don't know if it's still useful to keep the go syntax representation for clipboard context though for users who want to copy the string to go code.

hyangah avatar Oct 04 '21 14:10 hyangah

@polinasok @suzmue reminded that this was discussed in the context of https://github.com/golang/vscode-go/issues/1321. In particular, the debug variables section don't display multi-line strings well so, if \n is not escaped and if that's the first entry in particular, the result would be confusing. Current implementation also escapes other non-printable characters.

@gudvinr Do you have specific use case where go string representation isn't the best way to represent it? We can consider to provide an option to avoid escaping.

hyangah avatar Oct 04 '21 18:10 hyangah

@gudvinr Do you have specific use case where go string representation isn't the best way to represent it? We can consider to provide an option to avoid escaping.

JSON inside []byte is pretty valid use case. Since quotes inside string fields in JSON must be escaped, using this kind of output without prior knowledge leads to thinking that slice contains escaped JSON string. It's very frustrating that content inside string differs from content inside same slice.
It would be not that bad if \" were highlighted as it is inside editor I think. But still, it is quite annoying to constantly remind yourself that this escape sequence isn't part of original data when original data uses same escaping rules.

This is similar to json.RawMessage which is encoded as is but if you change type to []byte, json encoder will escape its content.

gudvinr avatar Oct 04 '21 19:10 gudvinr

Note that from the debugger's point of view, []byte is just []byte. The debugger has no knowledge on how this would be used (e.g. JSON or binary file or string or just byte) inside the code. I don't know what json.RawMessage has to do here - debugger just simply reads the memory location, checks the type (from debug info), and interpret the byte streams read from the memory. If this behavior is configurable, how do you want non-ASCII code, unicode, and \n to be presented?

hyangah avatar Oct 04 '21 20:10 hyangah

@hyangah I mentioned json.RawMessage as an example of data where string representation representation of []byte really matters because if you try to display json.RawMessage in debugger and see escaped data, it wouldn't be unreasonable to assume that these are what exactly encoded there since it's what encoding/json do. I didn't mean that it should mean something for debugger.

If this behavior is configurable, how do you want non-ASCII code, unicode, and \n to be presented?

This is a good question. I had to think about that.
One option is to output string in a way that is compatible with raw string. But in this case you can't easily see non-printable characters and that's not really good. If we do accept that visual representation won't be a valid string from Go perspective, then I'd like to see characters as is and those that can't be expressed as visually recognizable (e.g. \n, \r, \t and other ASCII/unicode symbols like that) would be instead shown as their escaped counterparts (like \n\tabc = "xyz\u200b").
I think the first option is better because it's convenient for more people. But it makes strings with apostrophes in them non-copyable so I don't think that should be default option.

gudvinr avatar Oct 05 '21 23:10 gudvinr