-
Notifications
You must be signed in to change notification settings - Fork 86
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
Problematic IsString
instance
#515
Comments
Orphan instances is a source of all kinds of problems. On the other hand, defining a |
Fair enough! In which case, I would advocate for the instance to either be removed, or instead placed behind a newtype itself. Library authors should, for sure, not create an orphan (as to not impact downstream users), but downstream end-users should be free to if they wish: it avoids a whole bunch of unnecessary boilerplate. |
So, you are requesting a backward incompatible change that might break other people's code so that you could avoid writing "boilerplate" in the form of a single-liner defining a |
Backwards incompatible changes by definition break code: I wouldn't be saying do it immediately, but if you're making a different breaking change that already necessitates changes in user code, then I'd be in favour of removing the instance at the same time. I.e. whenever you'd have otherwise made megaparsec 10. |
Megaparsec defines an instance:
Which allows for convenient syntax for parsing string symbols in a parser. However, the existence of this instance means that it is not possible to define a specialised instance for use with lexing logic (see this paper, page 8, section 3.3) without resorting to
newtype
hackery. This is arguably a more useful instance to have, because it can actually be used to clean up the logic of a completed parser.Obviously, the issue here is that the instance is given in the same module as
ParsecT
's definition (as to not orphan it), but I would suggest actually orphaning it in a module where it is the only thing inside. That way, users can opt into this (indeed very sensible) default, and remove the import when they want to define their own specialised version. If orphaning really is to be avoided, I'd instead just recommend removing it.In either case, this is a backwards incompatible change.
Keen to know your thoughts!
The text was updated successfully, but these errors were encountered: