Skip to content

Print Refs as short hashes in ambiguous hash error #6006

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

Jump to bottom
Merged
aryairani merged 2 commits into from
Nov 20, 2025
Merged

Print Refs as short hashes in ambiguous hash error #6006

aryairani merged 2 commits into from
Nov 20, 2025

Conversation

@ChrisPenner
Copy link
Member

@ChrisPenner ChrisPenner commented Nov 17, 2025

Overview

Old error message, which just uses Haskell Show instances for References, which isn't ideal because it doesn't show what you actually need to type.

@pchiusano/stepwise/main> alias.term #kids8s9hd2 new

  🤔

  The hash #kids8s9hd2 is ambiguous. Did you mean one of these hashes?

    Ref' (ReferenceDerived (Id "kids8s9hd2lo8moik96699cp7a8vt6v5dcksrtrkabf1c6fse1gsnnssrlf4avq5smn4m5vrka3slksdckd218ru8egremqmep3mt78" 0))
    Ref' (ReferenceDerived (Id "kids8s9hd2lo8moik96699cp7a8vt6v5dcksrtrkabf1c6fse1gsnnssrlf4avq5smn4m5vrka3slksdckd218ru8egremqmep3mt78" 1))

  Try again with a few more hash characters to disambiguate.

New message, which shows the correct syntax as a shorthash.

@pchiusano/stepwise/test> alias.term #kids8s9hd2 newish

  🤔

  The hash #kids8s9hd2 is ambiguous. Did you mean one of these hashes?

    #kids8s9hd2lo8moik96699cp7a8vt6v5dcksrtrkabf1c6fse1gsnnssrlf4avq5smn4m5vrka3slksdckd218ru8egremqmep3mt78
    #kids8s9hd2lo8moik96699cp7a8vt6v5dcksrtrkabf1c6fse1gsnnssrlf4avq5smn4m5vrka3slksdckd218ru8egremqmep3mt78.1

  Try again with a few more hash characters to disambiguate.

Implementation approach and notes

Just swaps the output printer.

Interesting/controversial decisions

Nah

Test coverage

Tested manually

Loose ends

It would be nice if all the ambiguity errors were just numbered outputs, but it looks like we don't have a returnEarlyNumbered or w/e so that's a slightly larger change, so I just did the quick thing for now.

@aryairani
Copy link
Contributor

aryairani commented Nov 17, 2025 •
edited
Loading

How much work would it be to support distinguishing #kids8s9hd2.0 and #kids8s9hd2.1 and then printing those? I kind thought we actually had code for that already.

Honestly we should probably print the .0 whenever there's cycle len > 1, so that we're not trying to distinguish #kids8s9hd2 (prefix) from #kids8s9hd2.1 anyway.

Edit: I'm guessing we don't have the cycle length everywhere we would want it though

@ChrisPenner
Copy link
Member Author

ChrisPenner commented Nov 17, 2025

How much work would it be to support distinguishing #kids8s9hd2.0 and #kids8s9hd2.1 and then printing those? I kind thought we actually had code for that already.

I dunno tbh, this is a side-quest so I just did something quick.
It'd be trivial to change the shorthash printer to always include it in all spots, but not sure that's what we want.

@aryairani
Copy link
Contributor

aryairani commented Nov 20, 2025

How much work would it be to support distinguishing #kids8s9hd2.0 and #kids8s9hd2.1 and then printing those? I kind thought we actually had code for that already.

I dunno tbh, this is a side-quest so I just did something quick. It'd be trivial to change the shorthash printer to always include it in all spots, but not sure that's what we want.

I'd want it in all spots if the cycle length > 1

The problem is that when we parse it we don't know whether the version without .0 is ambiguous or not; but it would be kind of annoying if we had to add .0 on to everything when most definitions aren't cycles.

Agree. re numbered args. Thanks for the patch!

@aryairani aryairani merged commit b4e3ab6 into trunk Nov 20, 2025
1 check passed
@aryairani aryairani deleted the cp/better-ambiguous-message branch November 20, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mitchellwrosen mitchellwrosen mitchellwrosen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ChrisPenner @aryairani @mitchellwrosen