Conversation
amesgen
left a comment
There was a problem hiding this comment.
Nice, thanks! A variant of this PR would be to adapt the displayException implementation of OrmoluException, in which case the implementation of withNiceExceptions would stay as it is:
diff --git a/src/Ormolu/Exception.hs b/src/Ormolu/Exception.hs
index c7db297..38fb022 100644
--- a/src/Ormolu/Exception.hs
+++ b/src/Ormolu/Exception.hs
@@ -47,7 +47,8 @@ data OrmoluException
OrmoluFixityOverridesParseError (ParseErrorBundle Text Void)
deriving (Show)
-instance Exception OrmoluException
+instance Exception OrmoluException where
+ displayException = T.unpack . runTermPure . printOrmoluException
-- | Print an 'OrmoluException'.
printOrmoluException ::|
I didn't want to do that, as changing an instance implementation would be a breaking change. And given that that instance hasn't changed in 3 years, I was hesitant to do so. But if you'd like, I can change |
Maybe I am missing sth, but why would changing the returned string from |
|
Would it not fall under "Did the behavior of any exported functions change?" in the flowchart in https://pvp.haskell.org/#decision-tree? Since instances are global, I would imagine instance functions are treated the same as exported functions |
|
Thanks, interesting, was not aware of that, seems a bit strong to require a major version bump for any change in behavior of any exported function; in the most strict interpretation, that would mean that you can only fix (even minor) bugs in your functions by releasing a new major version. A probably more charitable interpretation would be that one should release a new major version when changing the behavior of an exported function in a way that users are likely to rely on significantly, which I don't think would be the case here. The prose above also does not mention this case as a trigger for breaking changes:
In any case, let's just wait and see what e.g. @mrkkrp thinks 😄 |
|
I think we should probably go with adjusting the implementation of
Which is not what we currently do, so let's correct it. I think it is acceptable to change the implementation, I imagine that not many people rely on this. |
323a839 to
2a08b6c
Compare
|
@mrkkrp done |
No description provided.