Add support polykind for `Quote` from `Prim.TypeError` by xgrommx · Pull Request #4142 · purescript/purescript

xgrommx · 2021-07-08T22:34:35Z

Previously Quote working only with kind Type. Now it will be support any kind

For Example

data X :: forall k. k -> Type
data X a

foreign import data MkX :: forall k. k -> X k

e = ("a" :: forall a. Fail (Beside (Text "Some problem with ") (Quote (MkX Number))) => String)

main = log e

Will be produce

  A custom type error occurred while solving type class constraints:

    Some problem with MkX @Type Number

hdgarrood · 2021-07-09T05:19:57Z

tests/purs/failing/ProgrammablePolykindedTypeErrorsTypeString.out

+
+  A custom type error occurred while solving type class constraints:
+
+    Don't want to show Just @Type String because.


I think we should probably strip the kind annotations out here (the @Type bit).

We still want to show them when --verbose-errors is set, right?

If we already decided on a strategy of showing kind annotations when verbose errors is on, then applying that same strategy here sounds good. I can’t remember whether that should be happening anyway; do the tests run with verbose errors on?

As far as I can tell, in the only places we're currently erasing kind annotations in error messages, we do so conditional on verbose errors being off. I believe the tests don't run with verbose errors, which maybe makes testing that the erasure is conditional on verbosity difficult.

Also, it seems there are plenty of other places where we don't do this and perhaps should (just grep for @ in tests/**/*.out), which maybe makes dumping this request on this PR a little unfair? I agree that it should be done, but maybe we should clean this up holistically (and figure out how to test it robustly) in a separate PR.

Ok, yes, that makes sense.

We don’t strip them in the error “trace” to aid debugging.

Hiding kind annotations can happen separately.

rhendric · 2021-07-12T21:56:49Z

@xgrommx, could you please:

Rebase this on top of current master.
Create a file in CHANGELOG.d for this change—it doesn't have to be more than a brief bullet point. (See CHANGELOG.d/README.md, after you've rebased.)

When that's squared away, we can merge!

xgrommx · 2021-07-14T08:21:10Z

@rhendric what do u mean about rebase?

JordanMartinez · 2021-07-14T13:46:54Z

@xgrommx he means git rebase. We've changed how we handle the changelog (see #4132), so the rebase needs to occur so that you can add a file in that directory.

git fetch upstream
git rebase upstream/master fix-type-error-quote
# add a file to CHANGELOG.d (see its README.md)
# to document this change

JordanMartinez · 2021-07-23T14:27:44Z

@xgrommx Any updates on this?

xgrommx · 2021-07-24T23:48:26Z

@JordanMartinez sorry I was in vacation.

xgrommx · 2021-07-24T23:52:13Z

It's ok?

purescript-2:fix-type-error-quote* λ git fetch origin
purescript-2:fix-type-error-quote* λ git rebase origin/master fix-type-error-quote
Current branch fix-type-error-quote is up to date.
purescript-2:fix-type-error-quote* λ

JordanMartinez · 2021-07-25T04:32:10Z

@xgrommx Ah, no worries about that. I hope you enjoyed your vacation!

Your example above is fetching data from origin (i.e. your fork of purescript), not upstream (i.e the actual purescript repo). Similarly, it's rebasing against your fork's master branch, not the purescript repo's master branch.

Below is what I get when I run git remote -v, What do you get when you run that?

$ git remote -v
origin	git@github.com:JordanMartinez/purescript.git (fetch)
origin	git@github.com:JordanMartinez/purescript.git (push)
rh	git@github.com:rhendric/purescript.git (fetch)
rh	git@github.com:rhendric/purescript.git (push)
upstream	https://github.com/purescript/purescript.git (fetch)
upstream	https://github.com/purescript/purescript.git (push)

JordanMartinez · 2021-11-16T06:47:08Z

Rather than rebasing on master, I merged with master and added a changelog entry. I haven't tested this out, but I assume CI should still pass.

Make Quote from Prim.TypeError polykinded

be98777

JordanMartinez approved these changes Jul 9, 2021

View reviewed changes

hdgarrood previously requested changes Jul 9, 2021

View reviewed changes

hdgarrood approved these changes Jul 9, 2021

View reviewed changes

JordanMartinez added 2 commits November 15, 2021 22:43

Merge remote-tracking branch 'upstream/master' into fix-type-error-quote

3f2001d

Add changelog entry

8870619

rhendric merged commit 6b2b2a9 into purescript:master Nov 17, 2021


		A custom type error occurred while solving type class constraints:

		Don't want to show Just @Type String because.

Conversation

xgrommx commented Jul 8, 2021

Uh oh!

hdgarrood Jul 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhendric Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

hdgarrood Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

rhendric Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

hdgarrood Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

natefaubion Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

rhendric commented Jul 12, 2021

Uh oh!

xgrommx commented Jul 14, 2021

Uh oh!

JordanMartinez commented Jul 14, 2021

Uh oh!

JordanMartinez commented Jul 23, 2021

Uh oh!

xgrommx commented Jul 24, 2021

Uh oh!

xgrommx commented Jul 24, 2021

Uh oh!

JordanMartinez commented Jul 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordanMartinez commented Nov 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hdgarrood Jul 9, 2021 •

edited

Loading

JordanMartinez commented Jul 25, 2021 •

edited

Loading