Store more information in RenderedCode / refactoring#2550
Conversation
hdgarrood
force-pushed
the
include-ident-namespace
branch
2 times, most recently
from
03bf32e to
9bb997e
Compare
I'm not sure I understand this description of "backwards compatible" 😉 - is it that the uploads will start failing, but any existing "old" content will still work then? |
| dePrim ty@(TypeConstructor (Qualified _ name)) | ||
| | ty == tyBoolean || ty == tyNumber || ty == tyString = | ||
| TypeConstructor $ Qualified Nothing name | ||
| dePrim other = other |
There was a problem hiding this comment.
Do we not want to remove Prim. qualification anymore? Or is this handled somewhere else that I didn't notice now?
There was a problem hiding this comment.
It is handled inside Pursuit, yeah. Really this function should never have been there at all, it was an oversight to include it. We want to keep this information in because we use it to generate links; see for example https://pursuit.purescript.org/packages/purescript-react/2.0.0/docs/React#t:KeyboardEvent - some of those are broken and I am fairly sure it is because of this function.
There was a problem hiding this comment.
Ah ok, that totally makes sense! It was probably in here originally for psc-docs.
There was a problem hiding this comment.
Oh just to clarify: psc-docs output shouldn't change as a result of this PR either. Also here's the part of Pursuit where this information is used, in case you're interested: https://github.com/purescript/pursuit/blob/681559794a31e3de18452124a718ab09c64aaec6/src/Model/DocsAsHtml.hs#L201-L216
There was a problem hiding this comment.
I think I wrote this code originally, and this module was mostly copied from Language.PureScript.Pretty.Types as it was at that time - of course this was the behaviour we wanted in prettyPrintType, but not what we wanted here, although I failed to realise that at the time.
|
Yeah. Basically we don't have to try to regenerate the entire pursuit database, which is nice because that's a pain. I guess it's not fully backwards compatible but it is backwards compatible in the way that's most important from our point of view ;) |
|
Got it 👍 |
|
Oh wait a minute, I'm talking nonsense. Old compilers will still be able to upload to Pursuit! |
hdgarrood
force-pushed
the
include-ident-namespace
branch
from
9bb997e to
0e8f0a1
Compare
* Changes the RenderedCodeElement type to preserve more information for creating links in RenderedCode values. In particular, the namespace of a symbol is now stored. * Refactor Docs.RenderedCode.Types so that the functions for creating RenderedCode values are easier to use and more type-safe. * Include information about the module that kinds are contained in in the RenderedCode renderer. * Split Docs.RenderedCode.Render into two modules: Docs.RenderedCode.RenderType and Docs.RenderedCode.RenderKind. The JSON format has changed but in a backwards-compatible way, so the Pursuit database will not need to be regenerated.
hdgarrood
force-pushed
the
include-ident-namespace
branch
from
9c48d66 to
04d7668
Compare
|
I force-pushed so that I could fix the erroneous commit message. Also no rush, but it would be nice to get this reviewed, as afterwards I can look at generating HTML docs in |
|
@garyb Did you already review this? |
|
Travis is passing now. |
|
Bump - is there anything I can do to make reviewing this easier? |
|
Thanks! |
|
Sorry about the slow review :) |
|
No worries, I appreciate that it was quite a big PR :) |
3 participants
creating links in RenderedCode values. In particular, the namespace of
a symbol is now stored.
RenderedCode values are easier to use and more type-safe.
the RenderedCode renderer.
Docs.RenderedCode.RenderType and Docs.RenderedCode.RenderKind.
The JSON format has changed but in a backwards-compatible way. So once
this is released and Pursuit is updated with these changes, old
compilers will not be able to upload to Pursuit, but Pursuit will still
understand JSON produced by old compilers.
This gets us most of the way towards fixing purescript/pursuit#271 and purescript/pursuit#119.