-
Notifications
You must be signed in to change notification settings - Fork 457
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
Refactor Debug log in analysis #7276
base: master
Are you sure you want to change the base?
Conversation
@@ -2255,7 +2234,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable = | |||
user has already written. Just complete for the rest. *) | |||
let newText = | |||
c.name ^ " {\n" | |||
^ (cases | |||
^ (cases |> List.sort String.compare |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't really explain this one but at least sorting should make this deterministic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in chat, the reason for this being an if statement in the first place is to make it easy to avoid doing unnecessary computation unless actually in debug mode.
While we can achieve both probably by re-adding Debug.verbose
as Debug.isVerboseMode
or similar, I'd still like to understand a little bit more about why this change should be made. So, what are we gaining here more specifically?
Okay, my rationale for this PR: I'll start with a brief rant—feel free to skip this part! Secondly, understanding the code can be quite challenging. Many functions and types would benefit from additional documentation comments. I would also advocate for more type annotations to facilitate navigation through the codebase. These issues often lead me to add numerous Now, onto some specific points:
In summary, I want a safe way to log messages when verbose. I prefer not to clutter the code with |
No description provided.