Opened 11 years ago

Closed 8 years ago

#2362 closed feature request (fixed)

allow full import syntax in GHCi

Reported by: Isaac Dupree Owned by: igloo
Priority: high Milestone: 7.0.1
Component: Compiler Version: 6.8.2
Keywords: ghci, import Cc: rturk@…, jcpetruzza@…, gwern0@…, cgibbard@…, nicolas.pouillard@…, rendel@…, mad.one@…, mjm2002@…, ghci@…, johan.tibell@…, vogt.adam@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

for example, GHCi currently allows "import Data.Map" but not "import Data.Map as Map" or (to demonstrate a fuller range of import syntax) "import qualified Prelude as P hiding (mplus)". (In GHCi, you'd probably only be interested in hiding/qualifying something like Data.Map that made it harder for you to use Prelude functions due to actual ambiguity -- or vice versa)

see http://article.gmane.org/gmane.comp.lang.haskell.cafe/41272

(P.S. I would also like everything in a module, such as data declarations, to work in GHCi also, but I suspect that's a more complicated feature request?)

Attachments (2)

2362.patch (78.4 KB) - added by SamAnklesaria 8 years ago.
2362tests.patch (65.2 KB) - added by SamAnklesaria 8 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 11 years ago by guest

Architecture: UnknownMultiple
Cc: ghci@… added
Keywords: ghci import added
Operating System: UnknownMultiple

Currently you can do this using a helper module, which does the required imports.

module Import where

import qualified Data.Map as Map
*Prelude> :load Import.hs
*Import> Map.insert ...

Of course, doing this in GHCi would be nicer.

comment:2 Changed 10 years ago by Isaac Dupree

perhaps somewhat related to #2345

comment:3 Changed 10 years ago by igloo

difficulty: Unknown
Milestone: 6.10 branch

comment:4 Changed 10 years ago by simonmar

Architecture: MultipleUnknown/Multiple

comment:5 Changed 10 years ago by simonmar

Operating System: MultipleUnknown/Multiple

comment:6 Changed 10 years ago by thoughtpolice

Given the example that :load can load up qualified names as seen above, I don't think this would be too hard to add; if this is to be done, the GHC API seems like the best place to make the change, so ghc-api clients can use it too (like lambdabot.)

Since everything is basically wrapped in a do block, should we just extend that to allow full haskell98-style imports? e.g.

*Prelude> import Data.ByteString.Char8 (pack)
*Prelude Data.ByteString.Char8> import qualified Data.ByteString.Char8 as B
*Prelude Data.ByteString.Char8 B> let x = pack "hi"
*Prelude Data.ByteString.Char8 B> B.length x
2
*Prelude Data.ByteString.Char8 B>

Or, should we only allow GHCi to import things this way? Perhaps by just making an :import command or extending :module to allow full import syntax? Something like,

*Prelude>:m +qualified Data.Text as T
*Prelude T>:t T.null
T.Text
*Prelude T>:m-T
*Prelude>

Trac #1895 seems to indicate that several people want this feature, so I'm wondering if anybody has any objections/sees any problems, or has a better idea? I'd be willing to try and work on this for 6.12 if we can get an idea of what we want.

comment:7 Changed 10 years ago by thoughtpolice

Cc: mad.one@… added

comment:8 Changed 10 years ago by simonmar

I think it should be part of the GHC API, as a generalisation of API used by :module (setContext). Perhaps setContext should take a [ImportDecl], and we'd also need a way to parse a String into an ImportDecl. We also need to think about how to handle import *M.

For the implementation itself, a good place to start seems to be RnNames.rnImports, which returns a GlobalRdrEnv - exactly what we need to put in the InteractiveContext.

comment:9 Changed 10 years ago by igloo

Milestone: 6.10 branch6.12.1
Priority: normalhigh

See also #1895

comment:10 Changed 10 years ago by simonmar

related: #3217

comment:11 Changed 9 years ago by simonmar

Cc: rturk@… jcpetruzza@… gwern0@… cgibbard@… nicolas.pouillard@… rendel@… mjm2002@… added

merging with #1895, as this ticket will subsume it.

comment:12 Changed 9 years ago by tibbe

Cc: johan.tibell@… added

comment:13 Changed 9 years ago by simonmar

Milestone: 6.12.16.14 branch

Not going to happen for 6.12.1.

comment:14 Changed 9 years ago by aavogt

Cc: vogt.adam@… added
Type of failure: None/Unknown

comment:15 Changed 9 years ago by igloo

Milestone: 6.14 branch6.14.1

comment:16 Changed 8 years ago by SamAnklesaria

Owner: set to SamAnklesaria

comment:17 Changed 8 years ago by SamAnklesaria

Owner: changed from SamAnklesaria to igloo

My patch separates ghci 'import' syntax from ':module' syntax: while ':module' behaves the same way it always has (not allowing full import syntax), 'import' now acts like the full haskell98 'import'. This means, however, that starred module name imports (that is, full top levels) are now not supported by 'import'. With ':module' still supporting this, though, I don't think this is a problem.

Because import commands are represented by ImportDecls, I modified the "ic_exports" field of InteractiveContext to take both modules and their corresponding import declarations, if any. Along with this change comes a modification of getContext, which must use the different "ic_exports" type.

comment:18 Changed 8 years ago by simonmar

Many thanks for the patch! I've done a patch review, there are a few things that need fixing, but nothing fatal.

 -> throwOneError (mkPlainErrMsg noSrcSpan (ptext (sLit "No module speficied for import")))

typo in the error message, and perhaps the message should say something like "parse error in import declaration"?

+                                       -- 'ic_toplev_scope', 'ic_exports' and 'ic_imports'

did that sneak in by mistake? I don't see ic_imports anywhere

+getImportDecl :: GhcMonad m => String ->  m (ImportDecl RdrName)
+getImportDecl expr = withSession $ \hsc_env -> hscImport hsc_env ("import"++expr)

Did you really want "import"++expr, with no space between? I'd drop the "import"++ bit entirely, I think, and call the function parseImportDecl (we already have parseName, which is similar).

- | ["import", mod] <- words stmt    = keepGoing' setContext ('+':mod)
+ | ('i':'m':'p':'o':'r':'t':mod) <- stmt    = keepGoing' (importContext True) mod

check for a space after "import", otherwise you'll catch "imported" etc.

What happens if the user imports the same module twice, with different import lists? They should accumulate, as in a source file, right? Does it work that way? What happens if you import a module multiple times with "import", and then say ":m -M"?

In setContext and remembered_ctx it feels to me like an import decl should be a different kind of CtxtCmd, rather than dealing with them separately at a higher level. I'm not sure about the interactions of having multiple :m and import commands here, it would be simpler if they were all dealt with together in playCtxtCmd. I realise this means changing the types a bit more radically, though.

comment:19 Changed 8 years ago by simonmar

Oh, one more thing: we should have test cases for the new features, including the tricky cases like multiple imports and combinations of imports and :m, and re-loading of source files to trigger the replay functionality.

comment:20 Changed 8 years ago by SamAnklesaria

Thanks for the comments. parseName relies internally on a function from HscMain that does just that: name parsing. I don't think it can handle import declarations, which is why I wrote hscImport. As I removed the "import" from the string the user entered (saving all the import parts in a remembered_ctx seems redundant), it needs to be prepended or the parser won't understand it. Importing twice with different import lists is handled like in source files. Any ":m" commands will overwrite import declarations for the same module. The problem with representing an import in a CtxCmd is that it has a completely different syntax, so storing it in the same structure seems awkward. A new patch should be up soon.

comment:21 in reply to:  20 Changed 8 years ago by simonmar

Replying to SamAnklesaria:

Thanks for the comments. parseName relies internally on a function from HscMain that does just that: name parsing. I don't think it can handle import declarations, which is why I wrote hscImport.

Yes - I'm just suggesting that instead of getImportDecl you call it parseImportDecl and make it parse a complete import declaration for consistency.

As I removed the "import" from the string the user entered (saving all the import parts in a remembered_ctx seems redundant), it needs to be prepended or the parser won't understand it.

My suggestion is to not remove it in the first place, that seems simpler.

Changed 8 years ago by SamAnklesaria

Attachment: 2362.patch added

Changed 8 years ago by SamAnklesaria

Attachment: 2362tests.patch added

comment:22 Changed 8 years ago by SamAnklesaria

Status: newpatch

comment:23 Changed 8 years ago by simonmar

Resolution: fixed
Status: patchclosed

Applied, thanks for the contribution!

Thu Jun 24 20:26:32 PDT 2010  amsay@amsay.net
  * trac #2362 (full import syntax in ghci)
  'import' syntax is seperate from ':module' syntax
Note: See TracTickets for help on using tickets.