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

Configure "find" behavior on Thing classes #394

Merged
merged 11 commits into from
Jan 30, 2024
Merged

Configure "find" behavior on Thing classes #394

merged 11 commits into from
Jan 30, 2024

Conversation

towerofnix
Copy link
Member

Yay! This is a follow-up to #386, and unlike that PR, it includes a few significant behavior changes.

There are a lot of changes in this PR; it'd be split into separate ones if possible, but all the core changes work together and can't meaningfully be separated from each other.

  • Remove hard-coded descriptions saved on find from #find, with one exception that doesn't actually correspond to any Thing subclass (find.listing).
  • Remove hard-coded mapping of find function to data array in the bindFind utility function.
  • Move the above information into a new property on Thing constructors, [Thing.findSpecs], which may define multiple "find" functions. These definitions are in the same format as before, except with one new property: bindTo controls the wiki data array bound in bindFind.
    • bindTo is a string, not, say, a function of wikiData, because no complex filtering or logic should be performed here; it's purely for controlling bindFind. Filtering remains available through the include function on a find spec.
  • Re-define find and bindFind in terms of [Thing.findSpecs], working with new utility functions getAllFindSpecs and findFindSpec.
    • find is now a Freakin' Proxy, and actually the first (non-debug) use of proxies in HSMusic, despite us knowing about 'em for years! It's done this way so that references like find.artist still have meaning before the Artist class even exists. We thought this was crucial because find functions are very often referenced as part of a Thing constructor's getPropertyDescriptors phase... but, well, you know, the class constructors (including their static [Thing.findSpecs] value) are extant by that point... but it's kept this way anyway for reference and ready availability.
    • The only function of the find proxy is to facilitate getting a unique, reliable identity for functions like find.album before Thing constructors are actually ready. This function will error when called if Thing constructors still aren't ready, but its identity is available for static storage right away. We try to reduce general overhead as much as possible; the identity itself is just (...args) => behavior(...args), where behavior replaces its own identifier with the usual output of findHelper(spec), once that spec is actually available.
  • Combine artistData and artistAliasData into one. This is analogous to how artTagData contains both normal art tags and content warnings; for consistency and simplicity, we're going with that. The usual find.artist function is adapted and will never match aliases, so stuff consuming that doesn't need to adapt at all, but listings and other data or content code that directly access artistData (or artistAliasData) now filter as appropriate, like uses of artTagData.
    • The main reason for this change, rather than separating artTagData into two arrays, is that artists and artist aliases share the same directory namespace — that's largely the point of aliases (at the moment), so that old URLs will redirect you to the right artist. It's trouble if an artist and an unrelated alias, or two unrelated aliases, share directories!
    • Aliases have some slightly funky logic for determining if their directory is matchable or not. See artist.js for details; the point is to avoid having conflicting directories where we already know the common directory belongs to the obviously correct original artist. (Two aliases sharing a computed directory is also OK and ignored, since both would end up redirecting to the original artist.) This is all a bit clunky and kind of counter to how directories are supposed to work anywhere else on the website, so there's absolutely room to revisit it later on, but this enables directory deduplication between artists and aliases while essentially preserving previous behavior.
  • General minor cleanups.
    • filterDuplicateDirectories is renamed to reportDuplicateDirectories and has all of its filtering code removed, because as of 940b2cb (no PR, sorry!?) we just cancel the build if any duplicate directories were found — filtering behavior isn't needed anymore. This also cleans up the way the internal aggregate error is handled — it's just closed and thrown like any normal aggregate-throwing function, and this is caught and displayed by upd8.js, instead of having upd8.js access and close the internal aggregate itself.
    • Artists now show if they're an alias (and if so, of whom) when inspected, which helps actually resolve duplicate artist/alias directories.

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