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

dap: strip package paths from variable types #3516

Closed
stefanhaller opened this issue Sep 30, 2023 · 9 comments · Fixed by #3535
Closed

dap: strip package paths from variable types #3516

stefanhaller opened this issue Sep 30, 2023 · 9 comments · Fixed by #3535

Comments

@stefanhaller
Copy link
Contributor

Now that we strip paths from function names in the stacktrace view (since #3500), I want the same for the types of variables in the variables view.

Here's a simple example. With a reasonably sized side bar, I can't see any variable names or values, only the beginnings of the paths:
image

This is how ridiculously wide I would have to make the side bar so that I can see variable names and values (impractical on a laptop screen):
image

This is what I would like to see instead:
image

I'd be interested in working on this, but it looks a lot less straighforward than #3500 was, so I'd need some guidance.

@aarzilli
Copy link
Member

I'd be interested in working on this, but it looks a lot less straighforward

It is. Gdlv does something like this but it doesn't work in all cases with generics, if it's the type is too complicated it gives up: https://github.com/aarzilli/gdlv/blob/master/internal/prettyprint/short.go

@stefanhaller
Copy link
Contributor Author

@aarzilli This is very helpful, thanks. Would you be ok with reusing that code here?

However, when I said I need help I didn't only mean the algorithm to shorten the types, but also where in the code to do this. I suppose it needs to be in various places inside of childrenToDAPVariables, but I'm a bit overwhelmed by the complexity of that code, to be honest.

@aarzilli
Copy link
Member

aarzilli commented Oct 1, 2023

@aarzilli This is very helpful, thanks. Would you be ok with reusing that code here?

yes

However, when I said I need help I didn't only mean the algorithm to shorten the types, but also where in the code to do this.

I think it's just getTypeIfSupported

@stefanhaller
Copy link
Contributor Author

I think it's just getTypeIfSupported

I had hoped so too, but unfortunately that's not enough.

@hyangah
Copy link
Contributor

hyangah commented Oct 7, 2023

cc @suzmue

@stefanhaller
Copy link
Contributor Author

I looked into this some more, and if I'm not mistaken this can't be fixed in service/dap/server.go alone. I think this needs to be done in service/api/prettyprint.go, and I'm very much lacking the overview over the delve code base to tell what consequences it would have to change code there. Maybe other callers of that code would also benefit from shortening the types names? I really have no idea.

It would be fantastic if a more experienced developer would have the time to look into this and give some guidance; like #3500, it would be a big usability improvement.

@aarzilli
Copy link
Member

aarzilli commented Oct 8, 2023

prettyprint is used in several places, it can't be changed unconditionally, but it would be fine to add a flag to it, to simplify type names.

@suzmue
Copy link
Contributor

suzmue commented Oct 11, 2023

This would definitely be a big improvement. We have a similar request filed in the vscode-go extension issue tracker: golang/vscode-go#2569

I have a similar wip change that removes the types altogether, but that may be a more controversial change. Shortening the type names would probably need to touch the same places in the code: master...suzmue:delve:vartyps

@stefanhaller
Copy link
Contributor Author

Here's a PR: #3535

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants