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

no need to hide 'catch' #1309

Merged
merged 1 commit into from May 19, 2014
Merged

no need to hide 'catch' #1309

merged 1 commit into from May 19, 2014

Conversation

michaelt
Copy link
Contributor

This doesn't normally cause a problem because of some ghc workaround special to this case, but I was able to trigger an error with a complicated mixture of sandboxing, directing cabal to a locally installed ghc, and something else. catch isn't actually used in the file, so it seems it might as well go.

This doesn't normally cause a problem because of some ghc workaround special to this case, but I was able to trigger an error with a complicated mixture of sandboxing, directing `cabal` to a locally installed ghc, and something else. `catch` isn't actually used in the file, so it seems it might as well go.
@jgm
Copy link
Owner

jgm commented May 19, 2014

I believe the hiding is still needed for older GHC versions,
with older versions of base, in which pandoc exports 'catch'.
So the hiding should probably be kept but made conditional on
the MIN_VERSION_base using cpp macros.

@michaelt
Copy link
Contributor Author

My thought was that there can't be a problem, since Control.Exception isn't imported and no other catch is in scope. It seems catch was once used in this file but was dropped here b3ad94b#diff-8288955e209cfbead5b318a8598be9c0

I see that more than one approach is taken in pandoc. We find

pandoc/man/make-pandoc-man-pages.hs:import Prelude hiding (catch)
pandoc/src/Text/Pandoc/Writers/EPUB.hs:64:import Prelude hiding (catch)

where Control.Exception.catch is brought in scope; and

pandoc/src/Text/Pandoc/UTF8.hs:55:                       catch)

where it isn't (so the complicated cpp bit distinguishing older and newer versions of base) mustn't be necessary).

Nothing much hangs on it, but I could make a patch regularizing things to use the foolproof import qualified Control.Exception as E approach found in e.g.

 pandoc/pandoc.hs:1057:                           E.catch (UTF8.readFile tp')

and some other places.

@jgm
Copy link
Owner

jgm commented May 19, 2014

I commented without looking closely enough at your patch. You are right. The line isn't needed.
Thanks!

jgm added a commit that referenced this pull request May 19, 2014
@jgm jgm merged commit 0dd31b6 into jgm:master May 19, 2014
jgm added a commit that referenced this pull request May 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants