Sort hash maps in Settings display#10370
Conversation
| seaborn = sns, | ||
| tensorflow = tf, | ||
| tkinter = tk, | ||
| } |
There was a problem hiding this comment.
This is also now more consistent with our array printing.
crates/ruff/tests/snapshots/show_settings__display_default_settings.snap
Outdated
Show resolved
Hide resolved
| pub classes: FxHashSet<String>, | ||
| pub constants: FxHashSet<String>, | ||
| pub variables: FxHashSet<String>, | ||
| pub no_lines_before: FxHashSet<ImportSection>, |
There was a problem hiding this comment.
I changed these to use Fx for consistency with other settings, now that we sort in display anyway.
327d4b0 to
e8d8b04
Compare
8de82c6 to
667cb9b
Compare
|
|
I think the ecosystem check is a false positive? I see that violation on main and on this branch. |
667cb9b to
3883281
Compare
| write!($fmt, "{}{} = ", $prefix, stringify!($field))?; | ||
| if $settings.$field.is_empty() { | ||
| writeln!($fmt, "{{}}")?; | ||
| } else { | ||
| writeln!($fmt, "{{")?; | ||
| for (key, value) in $settings.$field.iter().sorted_by(|(left, _), (right, _)| left.cmp(right)) { | ||
| writeln!($fmt, "\t{key} = {value},")?; | ||
| } | ||
| writeln!($fmt, "}}")?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit: I think it would be good to move out as much code as possible from the macro. we could possibly do this by having a DisplaySet and DisplayMap struct that we would instantiate here (and call into) rather than implementing the logic inside of the macro. This has the added benefit that the logic can be reused elsewhere
Summary
We had a report of a test failure on a specific architecture, and looking into it, I think the test assumes that the hash keys are iterated in a specific order. This PR thus adds a variant to our settings display macro specifically for maps and sets. Like
CacheKey, it sorts the keys when printing.Closes #10359.