Skip to content

debug: display format for time.Time #1980

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

Closed
alialtun14 opened this issue Dec 29, 2021 · 8 comments
Closed

debug: display format for time.Time #1980

alialtun14 opened this issue Dec 29, 2021 · 8 comments
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge upstream-tools Issues that are caused by problems in the tools that the extension depends on.

Comments

@alialtun14
Copy link

The value of time.Time is displayed as follows.

time.Time {wall: 0, ext: 63775987200, loc: *time.Location nil}

Could it be converted the display format to a readable one?

@hyangah
Copy link
Contributor

hyangah commented Dec 29, 2021

Either evaluate from the DEBUG CONSOLE (call mytime.String()) or register the evaluation in the WATCH panel (example)

Let's use #158 for more than that.

Delve's call feature involves non-trivial complexity behind the scene (and AFAIK it is still classified as an experimental feature), so calling that for all the time.Time type variables automatically can be risky.

@hyangah hyangah closed this as completed Dec 29, 2021
@polinasok
Copy link
Contributor

This was an issue in delve repo. And I made the suggestion that this could be filed as an issue against vscode-go for us to consider adding a human-readable child as we did for bytes because this is not the first time our users requested help inspecting time vars.

@hyangah
Copy link
Contributor

hyangah commented Dec 29, 2021

That's possible only if dlv (and dlv dap) can reliably handle call. Until then, I don't see anything that can be done from the extension level. #158 is the general improvement feature request. If dlv can offer special expr evaluation formatting for time.Time or some well known types without going through the hoop of call, that will be great, but that's dlv issue.

@polinasok
Copy link
Contributor

We have been historically tracking/triaging dlv-dap issues (as experienced via the vscode UI) here as opposed to the dlv repo even though the changes were to be made to the dap server in the dlv repo. We can revisit that if you think these things are better tracked in the delve repo from now on.

#158 is so general (especially the customizable part), I don't see us working on that that any time soon. I think that's why we chose to special case bytes. I don't think we would want to use call, so we are in agreement there. But I thought maybe we could explore reconstructing time from the raw wall clock or whatever is stored in the struct itself.

@hyangah
Copy link
Contributor

hyangah commented Dec 30, 2021

But I thought maybe we could explore reconstructing time from the raw wall clock or whatever is stored in the struct itself.

That is the approach I am getting at and I think that's generally helpful for all delve users if implemented as extension of dlv's expression handling, not just for DAP users.

--
(EDIT) I am reopening this issue in case you plan to implement a DAP specific solution. Given that the original issue was reported as a general Delve issue, I hope the solution is available to general delve users and application beyond DAP, and the original issue is a better place to start the discussion among Delve devs. For example, if the solution could be available from the delve expression level, that would help users of watchpoint, display, and even just print command.

@hyangah hyangah reopened this Dec 30, 2021
@hyangah hyangah changed the title Display format for time.Time debug: display format for time.Time Dec 30, 2021
@hyangah hyangah added the Debug Issues related to the debugging functionality of the extension. label Dec 30, 2021
@hyangah hyangah modified the milestones: Untriaged, Unplanned Dec 30, 2021
@hyangah hyangah added the upstream-tools Issues that are caused by problems in the tools that the extension depends on. label Dec 30, 2021
@polinasok
Copy link
Contributor

polinasok commented Dec 30, 2021

My understanding is that the original issue was prompted by a debugging experience in VS Code, hence the redirect.
But, yes, I agree that it would be nice if such a feature was available easily in any delve client.

EDIT: Looks like this idea was already shut down in go-delve/delve#999.

@hyangah
Copy link
Contributor

hyangah commented Dec 30, 2021

If the only way to implement this is through call, I am also skeptical about automatically formatting time.Time.
If you plan to implement this bypassing call but interpreting the raw data directly, note that time.Time internal can change across Go releases (there is a precedent).

@hyangah
Copy link
Contributor

hyangah commented Jan 5, 2022

Closing in favor of go-delve/delve#999 which is now open again - we just need to make sure the new presentation is used in variables and evaluate requests.

@hyangah hyangah closed this as completed Jan 5, 2022
@golang golang locked and limited conversation to collaborators Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge upstream-tools Issues that are caused by problems in the tools that the extension depends on.
Projects
None yet
Development

No branches or pull requests

4 participants