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

Timing #46

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Timing #46

wants to merge 3 commits into from

Conversation

michaelmesser
Copy link
Contributor

Add timing

@ShinKage
Copy link
Contributor

Isn't better to add a command line option and/or an initialization option for timing?

@michaelmesser
Copy link
Contributor Author

There should probably be configuration for logging and timing. How exactly do you want it to look?

@ShinKage
Copy link
Contributor

Maybe we should inherit the compiler options at least for command line arguments, since the server effectively behaves as the compiler. So we add --log and --timing, we activate the relative options for the compiler, and we also use the same functionalities for our logging. However the compiler upstream may not be flexible enough at the current state. Therefore, maybe is better to at least have separate logging activate with the same flag, until we renconcile the mechanisms.

I'm not sure however if we should also add an option with the initialization message, since we are not really configurating anything, and is more a debugging option for developers than for users. What do you think?

@michaelmesser
Copy link
Contributor Author

Ideally there would be away to toggle logging levels without restarting the LSP

@ShinKage
Copy link
Contributor

I think the compiler provides the functionality. Then if we want to dynamically change the configuration is better to also provide an equivalent initialization option, and the client can send a didchangeconfiguration notification with the new configurations.

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.

2 participants