spago icon indicating copy to clipboard operation
spago copied to clipboard

Docs command: Fix how we get package names

Open CharlesTaylor7 opened this issue 2 years ago • 4 comments

Spun off from this conversation: https://github.com/purescript/spago/pull/1063/files#r1352609435

Here's what the docs html page looks like:

Screen Shot 2023-10-10 at 10 19 46 AM

There's a few issues, we'd like to address:

(1) Instead of throwing all the workspace modules under the heading <local package>, we ought to be able to use the actual package names for each package in the current workspace. They could still live in a separate section at the top of the page

(2) We don't need to clutter the view with version numbers.

(3) We'd like to make the implementation more robust than it is at the moment. Right now it's just directory matching on .spago/packages.

CharlesTaylor7 avatar Oct 10 '23 15:10 CharlesTaylor7

As mentioned in #1063:

I guess the easiest way to get this done would be to run a spago graph modules --json when we build the index, and dump the JSON in the docs folder, so the Halogen app can access this too.

That command is being implemented in #1069, so I'll mark this as blocked until that gets merged

f-f avatar Oct 11 '23 11:10 f-f

This is unblocked now: when we generate the index we can write to disk the result of spago graph modules --json in a place where the Halogen app can find it, and that lets us associate every module with its package (it looks like this)

f-f avatar Oct 15 '23 11:10 f-f

I think the next steps would be:

  • Determine a place where the halogen app can find the json file
  • Write the json file when generating the docs
  • Change halogen app: place all packages in current workspace
  • Change halogen app: while printing packages on the page, make a different section on the top for packages in the local package
  • Change halogen app: remove version numbers from names in index. (Have to check if the version number on on the package page though)

Quite some steps, perhaps it's not small?

Additionally

  • Define requirement "make implementation more robust". IMO it's better to postpone this until stuff starts breaking and someone reports a bug about it.

flip111 avatar Dec 24 '23 21:12 flip111

Define requirement "make implementation more robust"

This was coming from the perspective of "we have to match things in a folder and hope they are right". But now we can get the list of packages with spago graph, so it's a precise match. That's robust enough.

A few pointers:

  • the insertion point for writing the json file is here in line 51, before we build the index https://github.com/purescript/spago/blob/b450892d69aa3013861fde6c73299d2c17aba3dc/src/Spago/Command/Docs.purs#L51 ..where you'd call this with json: true: https://github.com/purescript/spago/blob/b450892d69aa3013861fde6c73299d2c17aba3dc/src/Spago/Command/Graph.purs#L68
  • it doesn't really matter where we stash this file, as long as the app can find it. generated-docs is the first place I would try.
  • This is where we figure out the package name from the folder name: https://github.com/purescript/spago/blob/b450892d69aa3013861fde6c73299d2c17aba3dc/docs-search/common/src/Docs/Search/Declarations.purs#L204-L214 We should instead read the file as a json when we start the app, and use the graph info from there to get the package name (this function is in the common package, but really it should be moved to the client-halogen package

f-f avatar Dec 25 '23 11:12 f-f