Skip to content
This repository was archived by the owner on Oct 17, 2024. It is now read-only.

Conversation

@koyeung
Copy link
Contributor

@koyeung koyeung commented May 12, 2024

This is to fix sort order among subtables, e.g. "tool.coverage.paths", "tool.coverage.report" and "tool.coverage.run".

Pls refer to the issue: #8 for details.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

We're supposed to keep the file order for these not reorder them lexicographic-ly.

Cargo.toml Outdated
pep508_rs = { version = "0.5.0" }
lexical-sort = { version = "0.3.1" }
regex = { version = "1.10.4" }
indexmap = "2.2.6"
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need an external package to solve this?

let mut order: Vec<String> = header_to_pos.clone().into_keys().collect();
// order headers by pos
let mut header_pos: Vec<(String, usize)> = header_to_pos.clone().into_iter().collect();
header_pos.sort_by_key(|&(_, v)| v);
Copy link
Member

Choose a reason for hiding this comment

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

Rather than sorting twice, can we just somehow append the position inside the store that happens immediately after this, that would be a better solution. Having two sorts make it look a bit suboptimal.

@gaborbernat gaborbernat merged commit 4913684 into tox-dev:main May 13, 2024
@koyeung
Copy link
Contributor Author

koyeung commented May 15, 2024

thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants