Skip to content

fix full type definition not being shown for new type defs in diff.update#6101

Merged
aryairani merged 3 commits intounisonweb:trunkfrom
bbarker:diff_update_type_fix
Jan 29, 2026
Merged

fix full type definition not being shown for new type defs in diff.update#6101
aryairani merged 3 commits intounisonweb:trunkfrom
bbarker:diff_update_type_fix

Conversation

@bbarker
Copy link
Contributor

@bbarker bbarker commented Jan 9, 2026

Overview

The diff.update command now shows the full structure of new types and abilities, not just their names.

Before:

New definitions:
  + ability Counter
  + structural type Person

After:

New definitions:
  + ability Counter where
  +   increment : {Counter} Nat
  +   getCount : {Counter} Nat
  + structural type Person = { name : Text, age : Nat }

This matches the behavior for updated types, which already showed their full structure with inline diffs.

Implementation approach and notes

Changed renderNewTypes in OutputMessages.hs to use DeclPrinter.prettyDecl (full declaration rendering) instead of DeclPrinter.prettyDeclOrBuiltinHeader (header only). This required threading TypeReferenceId through from DiffUpdate.hs and updating the ShowUpdateDiff type in Output.hs.

Test coverage

  • Added test case to unison-src/transcripts/idempotent/diff-update.md that covers new structural types and abilities

Loose ends

None.

Final checklist

  • PR title describes the change
  • Transcript demonstrates the changed behavior

@@ -2654,21 +2654,24 @@ notifyUser dir issueFn = \case
let ppe = PPED.suffixifiedPPE ppedNew
let colorAdd = P.green . ("+ " <>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this definition is no longer used, was that intentional? Or maybe it got copied somewhere else?

It's hitting "warnings as errors" in CI.

@bbarker bbarker force-pushed the diff_update_type_fix branch from ca7ab6a to 6833be0 Compare January 28, 2026 20:31
@aryairani
Copy link
Contributor

@bbarker I've been trying to coax this PR through too, but got interrupted. I think I can get it though, so don't worry.

For future PRs though, the gold standard is to run ./scripts/check.sh to prep it for submission. It seems like you have some custom stack settings though, that are preventing the issues from being flagged on your end ahead of time? Running the script may or may not address that, depending on the source of the custom settings.

Feel free to ping me on Discord if you wanna troubleshoot together in the future though.

@aryairani aryairani merged commit 72eda27 into unisonweb:trunk Jan 29, 2026
17 checks passed
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