search

nvim-neorocks / rocks

An alternative frontend app for luarocks.org
MIT License
24 stars 1 forks source link

fix(platform): remove `Ord` instance of `PlatformIdentifier` #14

Closed mrcjkb closed 7 months ago

mrcjkb commented 7 months ago

It really shouldn't have an Ord instance.

I'm not sure about how we sort platforms by specificity (see the comment I added).

mrcjkb commented 7 months ago

🤔 We should probably group and filter the platforms that can be sorted...

vhyrro commented 7 months ago

Is there a very concrete problem that could occur here? As long as there isn't I believe we'll be fine (famous last words). Or, in other words, as long as the more specific matches appear before the less specific matches, we don't really care about what order the rest will be in, right?

mrcjkb commented 7 months ago

Is there a very concrete problem that could occur here? As long as there isn't I believe we'll be fine (famous last words). Or, in other words, as long as the more specific matches appear before the less specific matches, we don't really care about what order the rest will be in, right?

The problem is that we don't have control over the order in which the rockspec specifies the platform overrides. If it has non-orderable platforms between the orderable ones, we can't sort them properly.

vhyrro commented 7 months ago

Perhaps we really should then partition the vector into specialized platforms and the regular platforms and just chain them together after the fact (such that the specialized platforms are guaranteed to be at the front). I'm pretty sure itertools has some crazy function to do all of that in one go :joy:

mrcjkb commented 7 months ago

Perhaps we really should then partition the vector into specialized platforms and the regular platforms and just chain them together after the fact (such that the specialized platforms are guaranteed to be at the front). I'm pretty sure itertools has some crazy function to do all of that in one go 😂

It ended up being a lot simpler :smile: