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

internal: remove spurious regex dependency #15071

Merged
merged 1 commit into from Jun 19, 2023
Merged

Conversation

matklad
Copy link
Member

@matklad matklad commented Jun 17, 2023

  • replace tokio's env-filter with a smaller&simpler targets filter
  • reshuffle logging infra a bit to make sure there's only a single place where we read environmental variables
  • use anyhow::Result in rust-analyzer binary

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2023
Logger { filter, file }
}

pub(crate) fn install(self) -> Result<()> {
// The meaning of CHALK_DEBUG I suspected is to tell chalk crates
Copy link
Member

Choose a reason for hiding this comment

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

The comment is a bit confusing now that the CHALK_DEBUG var is being read from somewhere else

crates/rust-analyzer/src/bin/logger.rs Outdated Show resolved Hide resolved
LspError, Result,
LspError,
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this refactor, I wonder if having a replace use with qualified name assist would be nice to have(reverse of replace_qualified_name_with_use)

- replace tokio's env-filter with a smaller&simpler targets filter
- reshuffle logging infra a bit to make sure there's only a single place
  where we read environmental variables
- use anyhow::Result in rust-analyzer binary
@matklad
Copy link
Member Author

matklad commented Jun 19, 2023

Landing as this seems somewhat conflict prone

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 19, 2023

📌 Commit 424ef77 has been approved by matklad

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 19, 2023

⌛ Testing commit 424ef77 with merge 00b9d9f...

@bors
Copy link
Collaborator

bors commented Jun 19, 2023

☀️ Test successful - checks-actions
Approved by: matklad
Pushing 00b9d9f to master...

@bors bors merged commit 00b9d9f into rust-lang:master Jun 19, 2023
10 checks passed
Comment on lines +55 to +56
let filter: tracing_subscriber::filter::Targets =
env::var("CHALK_DEBUG").ok().and_then(|it| it.parse().ok()).unwrap_or_default();
Copy link
Member

Choose a reason for hiding this comment

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

CI seems to be spamming the logs for this now

Copy link
Member

Choose a reason for hiding this comment

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

Actually not the CHALK_DEBUG part, we seem to log all salsa traces ...

Comment on lines -81 to +84
.with_env_filter(tracing_subscriber::EnvFilter::from_env("RA_LOG"))
// FIXME: I am not smart enough to figure out how to use this with
// `tracing_subscriber::filter::Targets`.
//
// .with_env_filter(tracing_subscriber::EnvFilter::from_env("RA_LOG"))
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess because of this? We now don't filter in slow tests so we get 30k lines of salsa logs 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants