Display inferred kind signatures in HTML docs by JordanMartinez · Pull Request #4119 · purescript/purescript

JordanMartinez · 2021-06-22T04:19:11Z

Description of the change

Fixes #4117.

Checklist:

Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
Linked any existing issues or proposals that this pull request should close
Updated or added relevant documentation
Added a test for the contribution (if applicable)

Work done so far:

The forall (k :: Type). k -> Type has been simplified to forall k. k -> Type.

Pending work

whether to display inferred kind signatures if all type parameters' kinds are Type (decision pending)
updating the changelog

JordanMartinez · 2021-06-22T14:20:12Z

Pretty sure this is ready for review. I still need to update the changelog to account for this PR.

hdgarrood · 2021-06-22T16:28:54Z

src/Language/PureScript/Docs/Convert.hs

+          kindFunctionType = P.TypeApp () kindPrimFunction kindPrimType
+
+          -- |
+          -- Returns True if the inferred kind signature follows this


How about calling this function isUninteresting and saying that an "uninteresting" kind is a kind of the form:

Type

Constraint

Type -> K where K is uninteresting.

I think that makes it clearer which kinds are included, and I think it will make the code a little easier to read.

Ah, thanks! That's a much better name.

hdgarrood · 2021-06-22T16:32:46Z

This looks great 👍 it looks to me like we will currently only hide uninteresting kinds if they have been inferred, so I think the only thing left to change is to additionally hide uninteresting kinds which are explicitly declared, if that's what we want to do (and I think it probably is, judging by the discussion in #4117?)

rhendric · 2021-06-22T16:09:58Z

tests/TestDocs.hs

+      , ShouldHaveKindSignature (n "KindSignatureDocs") "TShown" "data TShown :: (Type -> Type) -> Type -> Type -> Type"
+      , ShouldHaveKindSignature (n "KindSignatureDocs") "NShown" "data NShown :: Type -> (Type -> Type) -> Type -> Type"
+      , ShouldHaveKindSignature (n "KindSignatureDocs") "CShown" "data CShown :: (Type -> Type) -> Type -> Type -> Type"


As the failed build shows, these expectations should be changed to have the correct keywords.

Ha! Whoops!

rhendric · 2021-06-22T16:20:35Z

src/Language/PureScript/Docs/Convert.hs

+            P.TypeApp _ t1 t2 | t1 == kindFunctionType ->
+              isFunctionTypeStarTerminal t2
+            x
+              | x == kindPrimType || x == kindPrimConstraint -> True


The Constraint thing is a good point; I hadn't considered that classes have a different ‘most intuitive’ kind.

This || is correct because there's no way to make something Constraint-kinded that isn't a class right now, right? That is, something like

type IntState = MonadState Int

currently isn't supported, and there isn't anything else like that which could create a Constraint other than a class?

I'd feel a tiny bit more comfortable if this || were instead conditional on the declaration type—a x == if isTypeClass d then kindPrimConstraint else kindPrimType sort of deal—because some day type synonyms for constraints might be a thing? But perhaps that's unnecessary future-proofing, if we know for sure that there aren't any Constraint loopholes in the present.

That's a good suggestion - I think full constraint kinds (which includes being able to write type synonyms like your IntState one) is something that's likely to happen in the future, so I agree about making this conditional on the declaration type.

JordanMartinez · 2021-06-22T19:06:42Z

I think the only thing left to change is to additionally hide uninteresting kinds which are explicitly declared

Yeah, that's something I wasn't able to get to this morning due to running out of time.

JordanMartinez · 2021-06-22T19:55:08Z

Ok. Tests pass locally, and I believe I've addressed all comments. Let me know if there are any stylistic changes to make.

JordanMartinez · 2021-06-22T20:01:37Z

Now I just need to write the changelog...

JordanMartinez · 2021-06-22T20:01:59Z

Oh, and update the Pursuit page once this gets released.

rhendric · 2021-06-22T20:10:37Z

CHANGELOG.md

+  ```
+
+  The docs will include the kind signature for `Foo`, but will
+  intentionally remove the kind signature for `Bar`, even if `Bar`'s


Bar or Tuple?

Ah! Ran out of time again. Thanks for pointing that out.

hdgarrood · 2021-06-22T20:46:04Z

CHANGELOG.md

+
+  ```purescript
+                                                  {-
+  data Foo :: (Type -> Type) -> Type -> Type      -}


The compiler will infer forall k. (k -> Type) -> k -> Type for this data type, not (Type -> Type) -> Type -> Type.

Thanks. I decided to adjust this example to use (f Int) rather than (f a)

hdgarrood · 2021-06-22T20:47:17Z

CHANGELOG.md

+  `Type`. Thus, `Foo`'s inferred kind signature will be displayed
+  in its docs.
+
+  An "uninteresting" kind signature is one that follows this form:


I think we should say "kind" rather than "kind signature" here. A kind is something like Type, Type -> Type, or forall k. k -> Type. A kind signature is more than that: it's a declaration which tells you that a particular data type or type synonym or class has a particular kind, such as data Tuple :: Type -> Type -> Type.

Mm... Makes sense. I've rewritten parts of the changelog entry to keep this in mind.

JordanMartinez · 2021-06-23T00:51:24Z

I rewrote the changelog entry to use this organization. I thought it better presented the new feature in a clearer way.

kind signatures weren't displayed
kind signatures are now displayed (except those with 'uninteresting' kinds)
inferred kind signatures are used when explicit ones aren't provided
kind signatures with 'uninteresting' kinds (both explicit and inferred) are not displayed

JordanMartinez · 2021-06-23T19:03:32Z

If this is good to merge, can I get an approval?

hdgarrood · 2021-06-24T13:56:06Z

Please don’t rush people. I would still like to see @rhendric’s suggestion about making the “uninteresting” check depend on what kind of declaration we have implemented, and I would like to think a bit more about how we present this in the release notes.

JordanMartinez · 2021-06-24T15:08:41Z

Ah... Sorry about that.

hdgarrood · 2021-06-26T16:04:36Z

I think this changelog entry is a bit verbose. I don't think we should always provide examples of everything; that makes it difficult to quickly scan through a changelog to find what version a given change was made in. I think a more full description of how the feature works with examples to ensure that the reader understands would be better suited for the documentation repository. How about this instead:

The compiler now displays kind signatures for data, newtype, type
synonym, and type class declarations in generated documentation. The
compiler now also includes documentation-comments (i.e. those which start
with a | character) both above and below the associated kind signature
declaration (if any) in generated documentation, whereas previously
documentation-comments above a kind signature declaration were ignored.

Both explicitly declared and inferred kinds are included in documentation.
The compiler omits including a kind signature in generated documentation
only when the kind is considered "uninteresting". An uninteresting kind is
defined as one where all of the declaration's type parameters have kind
Type.

JordanMartinez · 2021-06-26T17:22:09Z

I think a more full description of how the feature works with examples to ensure that the reader understands would be better suited for the documentation repository.

Works for me!

JordanMartinez · 2021-06-26T17:27:08Z

I've added an issue to the docs repo, so we can add that page later.

JordanMartinez added 8 commits June 21, 2021 20:21

Test that inferred kind signatures appear in docs

3590b84

Insert inferred kind signatures into docs without restraint

dde0038

Update KindSignatureMismatch to output structure of inferred kind

19b8106

Replace 'forall (k :: Type). k -> ...' with 'forall k. k -> ...'

4c7cfac

Test: kind signatures where all type params have kind Type are hidden

ada0133

Don't show (Type ->)* -> Terminal kind signatures in docs

a090fff

Test that terminals don't have kind signatures either

9678209

Test that kind signatures with mixed kinds are displayed

1a2708b

hdgarrood reviewed Jun 22, 2021

View reviewed changes

rhendric reviewed Jun 22, 2021

View reviewed changes

Use correct keyword in expected text

a97c792

JordanMartinez added 4 commits June 22, 2021 12:10

Rename function to isUninteresting and update docs

8e1dda8

Remove explicit but uninteresting kind signatures; cleanup code

4bca719

Remove redundant pattern match

02c99a7

Fix final test

b3ffa11

Update changelog

9a627c2

rhendric reviewed Jun 22, 2021

View reviewed changes

hdgarrood reviewed Jun 22, 2021

View reviewed changes

Address changelog comments and rewrite parts of it

d40201d

Refactor to make it easier to adjust other declarations if ever needed

da6249c

Reduce verbosity of changelog entry

8c1c710

JordanMartinez mentioned this pull request Jun 26, 2021

[Wrong repo] Add an explanation on kind signatures appearing in documentation purescript/documentation#390

Closed

hdgarrood approved these changes Jun 26, 2021

View reviewed changes

JordanMartinez merged commit 962d94f into purescript:master Jun 26, 2021

JordanMartinez deleted the insertInferredKindSigsIntoDocs branch June 26, 2021 18:41

JordanMartinez mentioned this pull request Jun 26, 2021

Add an explanation on kind signatures appearing in documentation purescript/pursuit#434

Closed

JordanMartinez mentioned this pull request Jul 6, 2021

Still omit uninteresting kind signatures from HTML docs when redundant parenthesis are involved #4136

Closed

JordanMartinez mentioned this pull request Jan 18, 2022

Update version to v0.15.0 working-group-purescript-es/purescript#4

Closed

Conversation

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

Uh oh!

Uh oh!

JordanMartinez commented Jun 22, 2021

Uh oh!

hdgarrood Jun 22, 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!

Choose a reason for hiding this comment

Uh oh!

hdgarrood commented Jun 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanMartinez commented Jun 22, 2021

Uh oh!

JordanMartinez commented Jun 22, 2021

Uh oh!

JordanMartinez commented Jun 22, 2021

Uh oh!

JordanMartinez commented Jun 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanMartinez commented Jun 23, 2021

Uh oh!

JordanMartinez commented Jun 23, 2021

Uh oh!

hdgarrood commented Jun 24, 2021

Uh oh!

JordanMartinez commented Jun 24, 2021

Uh oh!

hdgarrood commented Jun 26, 2021

Uh oh!

JordanMartinez commented Jun 26, 2021

Uh oh!

JordanMartinez commented Jun 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JordanMartinez commented Jun 22, 2021 •

edited

Loading

hdgarrood Jun 22, 2021 •

edited

Loading