-
Notifications
You must be signed in to change notification settings - Fork 846
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Propose fuzzy-matched versions #557
Conversation
Fixes commercialhaskell#504. When you get a "Didn't see pkg-<ver> in your package indices" message, also see list of candidates with same major version. Might be that you forgot some minor thing like ".0" in the end of a version string.
import Network.HTTP.Download | ||
import Path | ||
import Prelude -- Fix AMP warning | ||
import Prelude |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment shouldn't get lost, as someone may then accidentally remove the import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snoyberg sorry, it was lost because of stylish-haskell, will bring it back
return $ Just bs | ||
|
||
-- TODO: use 'Maybe (NonEmpty PackageIdentifier)' return-type | ||
fuzzyLookupCandidates :: PackageIdentifier -> PackageCaches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be more efficient to convert PackageCaches
into a Map PackageName (Set Version)
to avoid traversing the entire collection for every mismatched name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snoyberg yeah, I decided to not go for efficiency, as this seems like a rare piece of functionality. But since you mentioned it – I have no problem making it efficient then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snoyberg I updated function to be better in terms of alg complexity. It now uses Map.splitLookup
to do a logN-split, filtering smaller elements. It then filters bigger elements by doing takeWhile
(takes only satisfying head).
Thus, I think I don't understand your comment in full. Since we're only running this algorithm for one package (I tested, if two packages didn't match we still fail and error only for the first one), the map-reconstruction for better version-matching wouldn't make a lot of sense. Might be that it would make sense in future, when we show errors for many packages at once.
I should note that I tested it on original repo with a problem: Before:
After:
|
Propose fuzzy-matched versions
I'm going to merge in now, sans the minor efficiency concern. Feel free to send another PR if interested :) |
Fixes #504.
When you get a "Didn't see pkg- in your package indices" message,
also see list of candidates with same major version. Might be that you
forgot some minor thing like ".0" in the end of a version string.