Steps toward pursuit 2#1067
Conversation
|
The build is failing because I was using |
psc-publish/ErrorsWarnings.hs
Outdated
|
Looks good. Could you please do me a favour and summarise what requests |
psc-publish/Main.hs
Outdated
There was a problem hiding this comment.
It's just occurred to me that this won't work if any purescript package author decides to use the main key in bower.json. This code assumes that bower list --paths --json prints a JSON object mapping package names to directory paths, which is not true if the main key is specified.
For example, purescript-timers#0.0.8 specifies main in its bower.json, and in the output of bower list --paths --json, you get:
"purescript-timers": "bower_components/purescript-timers/src/Control/Reactive/Timer.purs",
Instead, I think this code should just glob bower_components/* and then attempt to parse the second component as the package name.
There was a problem hiding this comment.
On second thoughts, we can get all the information we need from bower list --json.
Using bower list --json also allows us to easily identify bower dependencies which haven't been installed, because Bower includes "missing": true in the output where appropriate. So we can fail with a nice error if the user forgot to bower install.
|
At the moment, I was originally planning on using |
|
Come to think of it, given the HTTP client needs of this program are quite a lot simpler than I originally imagined, |
|
We don't necessarily have to use HTTP, that's fine. I'm just wondering, what can we get from the service that we can't get from the file? We could always require custom fields for the file. I'd prefer to have everything in one place honestly. That way, if Bower ever disappears, we can still keep the file format. |
|
Bower does have the So, we could check that the The downsides are:
See also https://github.com/paf31/psc-pages/pull/18, @bodil's comment in particular. |
|
We also aren't really getting everything from one place at the moment - a large chunk comes from the |
|
Well, given that we're going to create a cached snapshot of the docs at a particular version, the URL could become out of date at any point later anyway. At least with the Rather than parsing the |
|
Given that specifying a repository ought to be a very small amount of effort on the part of the maintainer however we choose to do it, and also given that we're happy assuming that PureScript packages are hosted on GitHub, I think I'd rather require this information be present, to ensure that we have source links in Pursuit documentation. I find them very useful for Haskell documentation on Hackage - and when they don't appear, I think it's quite irritating. I think if we want to know that we've got the right URL for the code at the time the docs were created, we're better off using the URL from the Bower registry, since this is the one that gets used when you Also, if we do want to get this information from the Bower registry, we could switch to |
|
I can have Pulp enforce the presence of this information to some extent as well. Absent the ability to reject published packages lacking it, that's probably the closest we'll get to a guarantee. |
|
Sorry, the presence of which information? |
|
Specifying a repository, in particular. |
|
Ok then - let's take it from |
|
I've fixed the issue stemming from |
|
Oh, one last thing I forgot, but would like to do - make all of the errors pluralise properly. The errors that refer to a list of things should use the singular if the list only has one thing in it. |
|
Ok, I'm really done now. :) |
|
Oh, also, I suppose we should warn people that |
|
Getting ready to merge this. Code looks great (@garyb, any comments?), but when I build, I pull in a lot of dependencies, including |
|
Not sure - I'll take a look. |
|
Looks like |
|
Looks like it's used by the binary. How disappointing. |
|
Ahhh, ok. I did waver about whether the |
|
So I compiled and ran this with |
|
I'm not really bothered about the size of the JSON files - With respect to how human-readable it is, it's admittedly not great. Running it through a JSON formatting tool does make it a lot easier, though. Also, I think ease of encoding/decoding is much more important, and I can't immediately see a good way of improving the verbosity without adding a fair bit of complexity to the encoding/decoding steps. |
|
Makes sense 👍 |
|
@paf31: Sorry, I've not really been keeping track of the progress with this - if you're fine with it, I am! |
|
I've just realised I've introduced a bug in The obvious fix is to require Alternatively, I could investigate modifying |
* Include a large chunk of code from psc-pages, allowing easier documentation output in multiple formats. * Rework psc-docs in order to use this new code. This makes psc-docs much simpler. * Partially fix #747, by putting instances in one chunk under their relevant data types and type classes. * Language.PureScript.Docs.RenderedCode now uses pattern-arrows in order to avoid unnecessary parentheses appearing in rendered code. * Remove unused code from psc-docs/Main.hs
|
That is a bit of a shame, but honestly, I think most people use a tool to generate docs anyway. The one case which would break is where we want to generate lots of small docs files, one per module. Maybe we need something analogous to I think desugaring imports etc. is the right way to go though. If we ever want to do things like add links in the generated Markdown, we need to do that. |
|
Ok, sounds good. I thought the same re |
|
Sounds good, but is that sort of thing easily supported with |
|
I'm not sure, I'll have a go and see what happens. |
|
Here's a draft changelog entry for these changes:
As far as I can tell, all we need to do now is:
|
|
I think putting the HTML code into the Pursuit server sounds fine. 👍 Also, before I forget, whatever new dependencies we add should be noted in the LICENSE file, since we're redistributing them in binary form. |
|
LICENSE file should be done now (see 719dde6). |
psc-publish/Utils.hs
Outdated
There was a problem hiding this comment.
Is it worth using the safe package for this?
There was a problem hiding this comment.
Yes, I think so. I'll add it
|
Reviewed, looks great. Sorry this took so long. Could you please squash commits, and I'll merge this in? |
Fixes #747 In addition to its previous behaviour of filtering out declarations which are not exported, `exportedDeclarations` will now also: * Filter out instance declarations which refer to non-exported types, as these can be considered as unexported. * Descend into data declarations, and filter out unexported constructors The effect of this is that tooling like psci and psc-docs can now simply call `exportedDeclarations` and not have to worry about anything.
Adds a new program which collects all information necessary for publishing a version of a package on pursuit, and dumps it to stdout as JSON. Included changes: * expand rendered docs types (Language.PureScript.Docs.Types) * add ToJSON instances and aeson-better-errors parsers * add ToJSON/FromJSON tests for UploadedPackage * look through devDependencies as well as normal dependencies in order to decide which dependencies are undeclared * make undeclared dependencies a warning, not an error It turns out making undeclared dependencies an error can be quite annoying - for example, when trying to run psc-publish on purescript-prelude, I had purescript-eff installed and undeclared (for testing), and it was failing because of this. Arguably, this should have been an error, and I should have added purescript-eff to devDependencies. But, I didn't want to do that, because there's no suitable version of purescript-eff to use yet. There are probably other situations where it would make sense to install packages for development locally and not add them to devDependencies too, which just haven't occurred to me. Given this, I think emitting warnings is enough; I think the strategy of throwing errors would carry a much higher risk of just being annoying than actually preventing errors.
This will allow parseAndDesugar to be reused in psc-docs.
psc-docs now requires all modules to be passed on the command line so that it can performs the desugarImports step of desugaring. This is necessary for it to be able to work out which instances should be displayed in the output. Accordingly, a new --docgen option has been added, similar to psc's --codegen, which allows you to specify the modules you want documentation for. See psc-docs --help for more information. This commit also removes the 'Module Documentation' header.
* Add packageName function, as a small convenience * Remove HTML generation from Language.PureScript.Docs It's moving into Pursuit server for now. * parameterize the Language.PureScript.Docs.Types.Package type so that packages with and without an uploader have different types, allowing stronger guarantees.
Also add Safe package
|
Is that ok? |
|
👍 x 100, thanks! |
|
Hooray :) |
5 participants
Fixes #747, continues from #1041, #1055. Much of this code comes from psc-pages.
The idea with the
AsHtmlmodule is that it is general enough that it could potentially be used in different environments with different stylesheets and page hierarchies - the rest of the code required to display documentation as HTML (eg including style sheets, templating) should end up in the pursuit server, I think.I renamed
psc-upload-packagetopsc-publishsince it doesn't upload anything.I commented on #1055 that I thought this tool should check for dependencies which aren't declared in bower.json, but I can't see an easy way of handling transitive dependencies. That is, if package A exists in bower_dependencies but is not listed in bower.json, we would need to tell the difference between A being pulled in as a transitive dependency of one of the other packages (which is OK), and A not being a transitive dependency of any other package, but having just been
bower install-ed without being added tobower.json(which is an error on the part of the maintainer, and could cause issues for users of that package). Given that this tool only exists to publish package information, and given that it would be quite complex, I no longer think this check should be in scope forpsc-publish.