Skip to content

refactor: Make keep-sorted debug output a bit more useful for figuring out why things were sorted a certain way. #88

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

Merged
merged 15 commits into from
Aug 14, 2025

Conversation

JeffFaer
Copy link
Collaborator

@JeffFaer JeffFaer commented Jul 14, 2025

The debug output now reports all of the tokens that were actually used
in comparisons. It also now reports all of the line groups in
their sorted order instead of the line groups in their original order,
but I think that should be fine.

If you want to see it in action, run something like

$ go run . -vv --id keep-sorted-test - < <(cat goldens/*.in) 1>/dev/null

JeffFaer added 6 commits July 14, 2025 10:20
I think "regexMatch" makes more sense than "regexToken" because we
aren't really doing anything special to the raw regex match.

I like "regexDidNotMatch" better than "alwaysLast" since the former
actually describes how we get that value instead of the business logic
implications.
This also moves the logic a bit more towards an imperative style from
the current functional style, which might be a bit easier to reason
about moving forward.

Also note that I'm caching calculated values and keeping track of which
values are calculated. Caching the calculated values should perhaps
improve performance (not that we've had any user complaints about
speed). I intend to use the fact that we're keeping track of which
values are calculated to improve the debuggability of keep-sorted in a
follow-up.
out why things were sorted a certain way.

The debug output now reports all of the tokens that were actually used
in comparisons. The debug output now reports all of the line groups in
their sorted order instead of the line groups in their original order,
but I think that should be fine.
@JeffFaer JeffFaer requested a review from KatrinaHoffert July 14, 2025 18:38
@JeffFaer JeffFaer changed the base branch from main to jfaer/debug2 July 14, 2025 18:39
Base automatically changed from jfaer/debug2 to main August 14, 2025 16:50
@JeffFaer JeffFaer merged commit 304f054 into main Aug 14, 2025
8 checks passed
@JeffFaer JeffFaer deleted the jfaer/actual_debug branch August 14, 2025 16:59
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