Skip to content

Conversation

@f-f
Copy link
Member

@f-f f-f commented Jan 16, 2020

So I patched purs to include Prim modules when generating the docs.json files: purescript/purescript#3769

It turns out we were assuming the sourceSpan was always there, but it's null in the case of builtins (which makes sense), so with this PR we'll consider something a "builtin" when there' no sourceSpan available.

Demo:

image

@f-f f-f requested a review from klntsky January 16, 2020 11:15
Copy link
Collaborator

@klntsky klntsky left a comment

Choose a reason for hiding this comment

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

Thank you! I refactored my code a bit.

@klntsky
Copy link
Collaborator

klntsky commented Jan 16, 2020

Also, I changed the string to be <builtin> instead of just builtins. It may not be clear that it is not from a package literally named builtins. And also there's no reason to diverge from Pursuit in such details.

@hdgarrood
Copy link

hdgarrood commented Jan 16, 2020

I think it might be preferable to use the same mechanism as the compiler here: a module is considered builtin if its first component is equal to "Prim", i.e. head (splitOn "." moduleName) == Just "Prim".

@klntsky
Copy link
Collaborator

klntsky commented Jan 16, 2020

I think it might be preferable to use the same mechanism as the compiler here

Nice idea, I didn't know Prim namespace was reserved.

We don't have access to module names as defined in source files here, though. We are just operating on file paths. And file names are not guaranteed to be the same as module names, so there may be false-positives.

@klntsky
Copy link
Collaborator

klntsky commented Jan 16, 2020

At the same time, I believe that nothing can go wrong when relying on sourceSpan === null.

@hdgarrood
Copy link

While I'm not aware of any case where this might fail now, it's definitely plausible that the compiler might fail to produce a source span for something which isn't builtin: perhaps as the result of a bug, or as the result of some code having been put together during desugaring or otherwise having been produced or rewritten within the compiler, such as via deriving.

@hdgarrood
Copy link

Don't you have access to the module name via the modName field at the top level?

@klntsky
Copy link
Collaborator

klntsky commented Jan 16, 2020

Don't you have access to the module name via the modName field at the top level?

Yep, we can use it, thanks.

@f-f
Copy link
Member Author

f-f commented Jan 18, 2020

@klntsky thank you so much for taking care of the fixes ❤️

I'll merge and cut a new release.

@f-f f-f merged commit 0b26229 into master Jan 18, 2020
@f-f f-f deleted the support-prim branch January 18, 2020 13:08
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.

4 participants