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.

Sign up for GitHub

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

feat(console): use tokio task ids in task views #403

Merged
merged 8 commits into from
Mar 28, 2023
Merged

feat(console): use tokio task ids in task views #403

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db7554a
feat(console): use tokio task ids in task views
hds Mar 13, 2023
e96be29
update clippy lint name
hds Mar 13, 2023
c5e9d69
Remove identity map
hds Mar 20, 2023
5e52bc5
And the formatting for that last change
hds Mar 20, 2023
2f2332e
Updated the task-id used in the async op table
hds Mar 22, 2023
a14fb75
Apply Eliza's suggestions from code review
hds Mar 27, 2023
d2e00c1
Keep cached version of task_id string for use in display
hds Mar 27, 2023
ce5c111
Merge branch 'main' into task-list-ids
hds Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions tokio-console/src/state/tasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ impl TasksState {
let id = ids.id_for(span_id);

let short_desc = strings.string(match (task_id, name.as_ref()) {
(Some(task_id), Some(name)) => format!("{} ({})", task_id, name),
(Some(task_id), None) => format!("{}", task_id),
(None, Some(name)) => format!("({})", name),
(Some(task_id), Some(name)) => format!("{task_id} ({name})"),
(Some(task_id), None) => task_id.to_string(),
(None, Some(name)) => name.to_owned()
Copy link
Member

Choose a reason for hiding this comment

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

missing , here (i think that was my fault, sorry!)

Suggested change
(None, Some(name)) => name.to_owned()
(None, Some(name)) => name.to_owned(),

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, rustc caught that one for me, I didn't notice it either. (-;

I needed to use name.as_ref().to_owned() here because name is an InternedStr. Not sure if there's a better pattern for this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, whoops, yeah, that makes sense. I assumed it was just a string.

(None, None) => "".to_owned(),
});

Expand Down