Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs command: Fix how we get package names #1065

Open
CharlesTaylor7 opened this issue Oct 10, 2023 · 4 comments
Open

Docs command: Fix how we get package names #1065

CharlesTaylor7 opened this issue Oct 10, 2023 · 4 comments

Comments

@CharlesTaylor7
Copy link
Collaborator

CharlesTaylor7 commented Oct 10, 2023

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.

@f-f
Copy link
Member

f-f commented Oct 11, 2023

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 f-f added this to the spago-next alpha bugs milestone Oct 11, 2023
@f-f f-f removed the blocked label Oct 15, 2023
@f-f
Copy link
Member

f-f commented Oct 15, 2023

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)

@flip111
Copy link
Contributor

flip111 commented Dec 24, 2023

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.

@f-f
Copy link
Member

f-f commented Dec 25, 2023

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
    liftAff $ IndexBuilder.run'

    ..where you'd call this with json: true:
    graphPackages { dot, json, topo } = do
  • 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:
    extractPackageName :: ModuleName -> Maybe SourceSpan -> PackageInfo
    extractPackageName (ModuleName moduleName) _
    | String.split (Pattern ".") moduleName !! 0 == Just "Prim" = Builtin
    extractPackageName _ Nothing = UnknownPackage
    extractPackageName _ (Just (SourceSpan { name })) =
    fromMaybe LocalPackage do
    topLevelDir <- dirs !! 0
    if topLevelDir == ".spago" then Package <<< PackageName <$> dirs !! 2
    else do
    bowerDirIx <- Array.findIndex (_ == "bower_components") dirs
    Package <<< PackageName <$> dirs !! (bowerDirIx + 1)

    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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants