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

debug formatting #7196

Merged
merged 5 commits into from
Aug 2, 2019
Merged

debug formatting #7196

merged 5 commits into from
Aug 2, 2019

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Jul 11, 2019

The compiler uses various records, and the default ToString fromatting used by a typical debugger ends up formatting these, which can lead to very slow debugging even when not inspecting these structures

@cartermp
Copy link
Contributor

Just to check, this doesn't prevent inspection of these structures if it's intentional?

@dsyme
Copy link
Contributor Author

dsyme commented Jul 11, 2019

Just to check, this doesn't prevent inspection of these structures if it's intentional?

That's right, it's just the debug text shown next to the value, you can still expand

@dsyme
Copy link
Contributor Author

dsyme commented Jul 11, 2019

BTW here's another ; you might want to get rid of in your output cleanup. {x = 1;}

> {x = 1}.ToString();;
val it : string = "{x = 1;}"

@cartermp
Copy link
Contributor

These dang semicolons

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dotnet/project-system
why did you not use the debuggerviewattribute to change the debugger output, since that is what it is for?

This PR appears to be almost all reformatting, with a couple of overrides added to change the debugging output. Do you think they should be split into a reformatting pr and a debug formatting pr?

src/fsharp/DetupleArgs.fs Outdated Show resolved Hide resolved
@dsyme
Copy link
Contributor Author

dsyme commented Jul 12, 2019

why did you not use the debuggerviewattribute to change the debugger output, since that is what it is for?

Because some other formatting (e.g. F# structured formatting) ends up using ToString, and we may as well just do this formatting everywhere that strings may come out - we never want to format these records as strings, it is arbitrarily expensive and impossible to read.

This PR appears to be almost all reformatting, with a couple of overrides added to change the debugging output. Do you think they should be split into a reformatting pr and a debug formatting pr?

No, I don't think it needs to be split, it's ok to combine a bit of reformatting with other changes that improve the codebase without changing external functionality.

@dsyme dsyme dismissed KevinRansom’s stale review July 30, 2019 13:54

Comment answered above

@KevinRansom KevinRansom merged commit 3e7cdd5 into dotnet:master Aug 2, 2019
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* debug formatting

* whitespace

* whitespace

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

Successfully merging this pull request may close these issues.

4 participants