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

Add ability to hide packages from search index #391

Merged
merged 1 commit into from
Jun 23, 2019

Conversation

klntsky
Copy link
Contributor

@klntsky klntsky commented Jun 5, 2019

Add ability to hide packages from search index by specifying pursuit-no-search-index as a bower keyword.

@hdgarrood
Copy link
Collaborator

hdgarrood commented Jun 6, 2019

Looks good so far, but I have a couple of suggestions:

  • "Hiding a package from the search index" is too low-level a description. I think this feature should be presented to users as "deprecating a package", and I think we should use pursuit-deprecated as the magic keyword.
  • I think we should additionally display a banner across the top of deprecated packages, a bit like what the npm registry does; see e.g. https://www.npmjs.com/package/bower. The banner should also appear for all versions, not just the version which has the pursuit-deprecated keyword. I think this can be achieved by adding a deprecatedPackages :: Set PackageName to the SearchIndex; a package should be considered deprecated across all versions if (and only if) the latest version has the magic pursuit-deprecated keyword.

If we do both of those I think we can consider #287 resolved.

@hdgarrood
Copy link
Collaborator

hdgarrood commented Jun 6, 2019

Oh also, in the help page, I think we should suggest that when deprecating a package, the package author should add a notice at the top of their README explaining why the package is deprecated and what previous users of it should now do instead.

@klntsky
Copy link
Contributor Author

klntsky commented Jun 6, 2019

newtype SearchIndex
  = SearchIndex { unSearchIndex :: Trie [IndexEntry] }

data IndexEntry = IndexEntry
  { entryResult  :: !SearchResult
  , entryType    :: !(Maybe D.Type')
  , entryRevDeps :: !(Maybe (Down Int))
  }

^ considering this I think that inserting the deprecated flag to IndexEntry instead of maintaining a set of package names will give us more elegant code. Moreover, it would be easier to hide every definition of a package, but not the package itself (not sure if it is a good idea).

@hdgarrood
Copy link
Collaborator

I’m not sure I understand why that would be more elegant - the reason I suggested the set of package names was that the only query we will want to do concerning this data will be “is this package deprecated or not” in order to decide whether to display the deprecation banner or not.

Now that you mention it, we probably should add a PackageResult to the index as well for deprecated packages, as I think searching for a deprecated package name should still succeed.

@hdgarrood
Copy link
Collaborator

Oh ok, I see what you're saying then; if we search for a particular package in the search index and check the deprecated flag, then we can store that information without having to add fields to SearchIndex. I'd suggest adding that field to the PackageResult constructor though, since I'd prefer not to store package deprecation statuses on non-package results (instead we can just omit them from the index). Another good thing about adding a field to the PackageResult constructor is that we can add deprecation markings in search results if we do it that way.

@klntsky
Copy link
Contributor Author

klntsky commented Jun 6, 2019

I'd suggest adding that field to the PackageResult constructor though

Yeah, I was thinking about the same a minute ago.

Will do.

@klntsky
Copy link
Contributor Author

klntsky commented Jun 11, 2019

Previews:

  1. Search results

2019-06-11-023301_502x295_scrot

  1. Module docs page

2019-06-11-023251_474x374_scrot

  1. Package page
    2019-06-11-023239_328x140_scrot

@hdgarrood
Copy link
Collaborator

Looks great, thanks! I've just realised that this won't play fantastically well with the caching mechanism (see Handler.Caching) because we currently only generate HTML for the docs for a particular package once, the first time that package is requested, so that might need a bit of additional work. I'm going to merge this now anyway because it's definitely an improvement.

For future reference, the way we currently deal with information which might change after a package has been uploaded for the first time is to provide that data separately and fetch it with XHR after the page has loaded. This is what the available-versions endpoint is for, for example. Concretely, if someone uploads version 1.0.0 of purescript-foo, we generate the HTML page for packages/purescript-foo/1.0.0 once, and then that page fetches the available-versions endpoint when it loads so that if someone later uploads version 2.0.0 we don't need to regenerate the entirety of the page packages/purescript-foo/1.0.0 in order to have version 2.0.0 appear in the version selector dropdown; we just need to regenerate the JSON object we provide via the available-versions endpoint.

So the obvious approach for addressing this would be to provide package deprecation statuses via a separate XHR endpoint in a similar way to how we provide available versions. However, I'm also open to replacing this caching mechanism entirely, since it's fairly awkward.

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.

2 participants