Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upQualify imports by default #1901
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jdegoes
Feb 28, 2016
A random idea: what if open imports are allowed, but they import only what a module has specifically denoted a default import item? i.e. give modules control over which of their terms are imported in an unqualified import.
One can imagine something like this:
module Foo where
( default Bar(..)
, default unBar
)
...
Then import Foo(..) becomes stable and non-breaking unless the library author screws up, which of course they can do anyway by replacing a release or just breaking semver.
jdegoes
commented
Feb 28, 2016
|
A random idea: what if open imports are allowed, but they import only what a module has specifically denoted a default import item? i.e. give modules control over which of their terms are imported in an unqualified import. One can imagine something like this:
Then |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
commented
Feb 28, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Feb 28, 2016
Member
^ that proposal is certainly another way to fix it, but I feel like it's even more complicated that the single-open-import approach. I think deciding what should be exported implicitly/explicitly might be hard to do at the point of authoring a module too... also unless we could enforce semver it seems more prone to the rules being accidentally broken.
|
^ that proposal is certainly another way to fix it, but I feel like it's even more complicated that the single-open-import approach. I think deciding what should be exported implicitly/explicitly might be hard to do at the point of authoring a module too... also unless we could enforce semver it seems more prone to the rules being accidentally broken. |
paf31
added
the
modules
label
Feb 28, 2016
paf31
added this to the Ideas milestone
Feb 28, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
Feb 28, 2016
Member
I like one of this proposal very much: that import Prelude only brings Prelude into scope, and that Prelude is not in scope by default. I've wanted that for a while, it's closer to Haskell, and it allows us to parse the module header in isolation to determine module dependencies, which will be a small performance improvement for rebuilds.
This means we should be able to write things like
import Control.Monad
import Control.Monad ((<=<))and have Control.Monad available for qualified use, but <=< in scope for infix use.
I dislike using open imports (..), in any form, for reasons I've given before.
Generally, I'd be willing to entertain another round of big changes to module imports, as long as we got something we were going to be very happy with, and ideally so happy that we wouldn't change it before 1.0. But I don't want to make a set of breaking changes just to experiment, so I'd want to be sure about whatever proposal we implemented.
|
I like one of this proposal very much: that This means we should be able to write things like import Control.Monad
import Control.Monad ((<=<))and have I dislike using open imports Generally, I'd be willing to entertain another round of big changes to module imports, as long as we got something we were going to be very happy with, and ideally so happy that we wouldn't change it before 1.0. But I don't want to make a set of breaking changes just to experiment, so I'd want to be sure about whatever proposal we implemented. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Feb 28, 2016
Member
I'm still not totally cool with removing the ability to access modules fully qualified without importing them first, I find the need to do so every time I want to use Debug.Trace in Haskell infuriating, but this proposal doesn't impose that requirement, it's just saying that if you do import X it's equivalent to import X as X. That may be a little redundant, but is a more sensible default than open-importing now we're discouraging that as being the common case.
Re: (..) we'd still need it for this single-open-import approach I'm working on / convenience during development / an easy way to find out what to replace it with when you want to do whole-module exports and the like.
|
I'm still not totally cool with removing the ability to access modules fully qualified without importing them first, I find the need to do so every time I want to use Re: |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
Feb 28, 2016
Member
As long as (..) gives a warning, I'm fine with it.
Are you already implementing single-open-import?
|
As long as Are you already implementing single-open-import? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Yep, hoping to have an initial PR for it tonight :) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
And it's just going to affect the warnings, right? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Feb 28, 2016
Member
Yeah, no warning necessary when there is only one open import, otherwise behave as before, as per #1869
|
Yeah, no warning necessary when there is only one open import, otherwise behave as before, as per #1869 |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Feb 28, 2016
I actually didn't consider the fully qualified case. @garyb Would it be much worse to add it to your project prelude that reexports stuff? Besides header parsing couldn't it encourage terse module naming to avoid explicit imports?
texastoland
commented
Feb 28, 2016
|
I actually didn't consider the fully qualified case. @garyb Would it be much worse to add it to your project prelude that reexports stuff? Besides header parsing couldn't it encourage terse module naming to avoid explicit imports? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
commented
Feb 28, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Feb 28, 2016
Member
This should be pretty easy to do as a first contribution... especially if we want to introduce it in a non-breaking, deprecate-first kind of way
- The
import Xsyntax will continue to work as is, but always warns that its meaning will be changed and instead should be written asimport X (..).
That means a tweak to the parser to allow (..) again, and then storing some kind of flag in the ImportDeclaration to determine whether the old syntax was used, so that later on during linting/name desugaring we can throw the warning.
It does mean that import X as X will still have to be used until we drop the old syntax entirely, but I think that's better than dropping it as a breaking change.
|
This should be pretty easy to do as a first contribution... especially if we want to introduce it in a non-breaking, deprecate-first kind of way
That means a tweak to the parser to allow It does mean that |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Feb 28, 2016
I'll start tomorrow
Besides header parsing couldn't [non-imported fully qualified names] encourage terse module naming to avoid explicit imports?
^ Valid concern?
texastoland
commented
Feb 28, 2016
|
I'll start tomorrow
^ Valid concern? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Feb 28, 2016
Member
I don't know, the main cases I have issue with is when I'm using things from Debug.Trace and a Unsafe.Coerce.unsafeCoerce unit hole value - things I only want to drop into the code temporarily and then delete later. It's nice not having to faff around with the imports every time you drop a trace/hole in, and then again later when you go back to remove debug code.
I don't have a "good" argument for not doing it, aside from it being annoying for the case I described above, and that it seems unnecessary (the performance boost can also be achieved by storing dependency info in the generated externs files).
|
I don't know, the main cases I have issue with is when I'm using things from I don't have a "good" argument for not doing it, aside from it being annoying for the case I described above, and that it seems unnecessary (the performance boost can also be achieved by storing dependency info in the generated externs files). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Feb 28, 2016
Would it be much worse to add [them] to your project prelude that reexports stuff?
^ Tenable solution?
texastoland
commented
Feb 28, 2016
^ Tenable solution? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Feb 29, 2016
@garyb After thinking further this would become even less of an issue (from experience using JetBrains products) with mature tooling. Perhaps we should start logging these ideas in @nwolverson's IDE core project? I added a note in the issue description to disallow fully qualified names.
texastoland
commented
Feb 29, 2016
|
@garyb After thinking further this would become even less of an issue (from experience using JetBrains products) with mature tooling. Perhaps we should start logging these ideas in @nwolverson's IDE core project? I added a note in the issue description to disallow fully qualified names. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
nwolverson
Feb 29, 2016
Contributor
Definitely tooling can help with imports, e.g. adding to the explicit imports list when inserting auto-completion, I think @kRITZCREEK was suggesting this also.
|
Definitely tooling can help with imports, e.g. adding to the explicit imports list when inserting auto-completion, I think @kRITZCREEK was suggesting this also. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Feb 29, 2016
@nwolverson Sweet pull me in if you want an extra pair of eyes! I plan on hacking on the Code plugin and Core after PS Conf. I'd love to hear an intro talk online around that time frame too
texastoland
commented
Feb 29, 2016
|
@nwolverson Sweet pull me in if you want an extra pair of eyes! I plan on hacking on the Code plugin and Core after PS Conf. I'd love to hear an intro talk online around that time frame too |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Mar 2, 2016
- Unimported fully qualify names (e.g.
Unsafe.Coerce.unsafeCoerce) may introduce name clashes between minor versions (like open imports in general), encourage terse module naming (e.g.Minstead ofControl.Monad) to avoid explicit imports, and obviate header parsing to optimize dependency analysis.
^ Added note about fully qualified names (I think error but arguably warning?), examples for import X (A) as X and import Y (..) hiding (f) as Y, and deprecation checklist.
texastoland
commented
Mar 2, 2016
^ Added note about fully qualified names (I think error but arguably warning?), examples for |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Mar 2, 2016
Member
What's the case that causes name clashes here?
Unimported fully qualify names (e.g.
Unsafe.Coerce.unsafeCoerce) may introduce name clashes between minor versions (like open imports in general)
I didn't think there was one, as it's just an explicit reference.
|
What's the case that causes name clashes here?
I didn't think there was one, as it's just an explicit reference. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Mar 2, 2016
Given a package Foo with a module Control.Monad.BazM, another package Bar could potentially add Control.Monad.BazM, creating ambiguity, unless I'm thinking wrong?
texastoland
commented
Mar 2, 2016
|
Given a package |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Mar 2, 2016
Member
Hmm, no you're not, but actually that's a problem whether we allow fully qualified references or not - it will result in a DuplicateModule error at the moment, with the compiler being unaware of packages.
|
Hmm, no you're not, but actually that's a problem whether we allow fully qualified references or not - it will result in a |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
Mar 2, 2016
Contributor
Yeah - that situation would always give a compile error, no matter what your imports say, unfortunately.
|
Yeah - that situation would always give a compile error, no matter what your imports say, unfortunately. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Mar 2, 2016
Ah go ahead and remove that reason then? What do you think about it being a warning? That wouldn't resolve @paf31's concern though.
texastoland
commented
Mar 2, 2016
|
Ah go ahead and remove that reason then? What do you think about it being a warning? That wouldn't resolve @paf31's concern though. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Mar 2, 2016
Member
Well, as I mentioned before, I don't really know what we gain by making it a requirement. It's not strictly necessary for dependency analysis when rebuilding, as we can use externs files for the same purpose if we stored the information in there.
If there's a good reason to do it, then I'm totally fine with it, but at the moment I haven't really seen anything that necessitates the change. Well, aside from the fact that it would be weird that you can either import or not modules that you reference fully qualified!
|
Well, as I mentioned before, I don't really know what we gain by making it a requirement. It's not strictly necessary for dependency analysis when rebuilding, as we can use externs files for the same purpose if we stored the information in there. If there's a good reason to do it, then I'm totally fine with it, but at the moment I haven't really seen anything that necessitates the change. Well, aside from the fact that it would be weird that you can either |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
commented
Mar 2, 2016
|
What about a warning because it's poor style and my naming concern? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
garyb
Mar 2, 2016
Member
Haha... I keep missing these obvious solutions
It would be perfect in fact, because the cases I mentioned where I think fully qualified imports are most useful are cases I'd never want left in finished code anyway.
|
Haha... I keep missing these obvious solutions It would be perfect in fact, because the cases I mentioned where I think fully qualified imports are most useful are cases I'd never want left in finished code anyway. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
commented
Mar 2, 2016
|
Great I'll change the description! I started working today. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
Mar 2, 2016
Member
We can't use externs, since the externs only get updated after the rebuild.
|
We can't use externs, since the externs only get updated after the rebuild. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
Mar 2, 2016
Member
Local imports like this are a convenience, sure, but they stop us from optimizing rebuilds, since we have to parse every module in full on every rebuild. For a very big project, that can add up to seconds. Sure, we can cache imports in a simpler file format for whenever the file didn't change, but more cached data just means more opportunities for stale cached data.
|
Local imports like this are a convenience, sure, but they stop us from optimizing rebuilds, since we have to parse every module in full on every rebuild. For a very big project, that can add up to seconds. Sure, we can cache imports in a simpler file format for whenever the file didn't change, but more cached data just means more opportunities for stale cached data. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Related #1511 |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Mar 10, 2016
Thanks I'll take a look! I started a week ago but have been busy preparing the meetup.
texastoland
commented
Mar 10, 2016
|
Thanks I'll take a look! I started a week ago but have been busy preparing the meetup. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
soupi
Mar 15, 2016
Contributor
One thing that is missing for me in this proposal is the ability to export only a few things unqualified. for example: import Prelude (id) will import id but not unit and not Prelude.unit. I'd like the ability to limit the import of things into scope, even when qualified.
|
One thing that is missing for me in this proposal is the ability to export only a few things unqualified. for example: |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Mar 16, 2016
@soupi You're correct that isn't supported. Could you explain why you want it? The only reason import X (A) as X has similar semantics is for the reexport use case in #1869 (comment).
texastoland
commented
Mar 16, 2016
|
@soupi You're correct that isn't supported. Could you explain why you want it? The only reason |
texastoland
referenced this issue
Mar 16, 2016
Closed
Require modules to be imported even if their members are being referenced fully qualified #1511
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
soupi
Mar 16, 2016
Contributor
I just didn't like the idea of not being able to limit what I want to import, but after some thought I can't really explain why. you can disregard my comment. I like the proposal overall :)
|
I just didn't like the idea of not being able to limit what I want to import, but after some thought I can't really explain why. you can disregard my comment. I like the proposal overall :) |
texastoland
referenced this issue
Apr 13, 2016
Closed
Permit a single open import without a warning #1869
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
mcfilib
Apr 13, 2016
@AppShipIt I really appreciate the way you spelled out this proposal in the issue description. Thank-you.
mcfilib
commented
Apr 13, 2016
|
@AppShipIt I really appreciate the way you spelled out this proposal in the issue description. Thank-you. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
@kRITZCREEK ah my mistake. Thanks for the correction |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Apr 29, 2016
@jplatte: Also I don't see why you would want to increase the amount of imports that will need a change to almost all of them by making import X (A) a qualified import.
me: I originally requested an incremental change (n.b. brevity of Deprecation) to make common uses cases easier: 1) warning on bare imports and 2) verbose aliases.
@kRITZCREEK: Why take the most concise syntax and occupy it with a use case only important for selective reexports.
@paf31: I think we should be careful to change things just for the sake of it.
@LiamGoodacre Thanks for an intriguing idea! Any given breaking change (even pre-1.0) should address a felt problem. For the reasons quoted above I can't advocate a fundamental change.
I may take a stab again this weekend. I'm updating the description but concrete changes would be:
import X (..)⇠import X(to make open imports explicitly opt-in)import X⇠import X as X(to make importing top-level namespaces likeNeoneasy)import X hiding (f)only exposesAand notX.f(to make imports more explicit #1901 (comment))import Xrequired to exposeX.Afully qualified (for module header parsing #1901 (comment) #2054)
Criticisms:
import X (..)would breakimport Preludein existing code which doesn't warn anyway thanks to @garyb.import X.Y.Zwould importX.Y.Zfully qualified which only helps tersely named modules likeNeon.- Haskell's use case for exposing
Xfully qualified is accessing hidden names likePrelude.bindinimport Prelude hiding (bind)for rebindabledonotation. - There's a case to be made everything works fine today (except for module header parsing).
Mind voting
texastoland
commented
Apr 29, 2016
•
@LiamGoodacre Thanks for an intriguing idea! Any given breaking change (even pre- I may take a stab again this weekend. I'm updating the description but concrete changes would be:
Criticisms:
Mind voting |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tfausak
May 2, 2016
Contributor
This is a huge thread. If @AppShipIt's latest comment is the start of the art, I love it.
I have one idea I'd like to float while we're talking about changing imports. I want import X.Y to mean import X.Y as Y. That is, I think the implicit qualified name should only be the last part of the module name. I do this way too often:
import Control.Monad as Monad
import Data.List as List
import System.Environment as Environment@kRITZCREEK gave a good counterexample to this. Some modules end with the same thing, like Control.Monad.Reader.Trans and its Writer counterpart. I think changing the default qualified import would encourage people to put something unique at the end. It would also allow people to continue using the Control, Data, System, et al. hierarchy without making imports tedious.
|
This is a huge thread. If @AppShipIt's latest comment is the start of the art, I love it. I have one idea I'd like to float while we're talking about changing imports. I want import Control.Monad as Monad
import Data.List as List
import System.Environment as Environment@kRITZCREEK gave a good counterexample to this. Some modules end with the same thing, like |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 2, 2016
If @AppShipIt's latest comment is the start of the art,
Yeah I need to update the description again. I'd like to wrap up because it's been pretty thoroughly explored.
I want
import X.Yto meanimport X.Y as Y.
That sounds right to me since @kRITZCREEK's example would probably be aliased anyway. Does the period have any semantic meaning besides an allowed character in the module name?
texastoland
commented
May 2, 2016
•
Yeah I need to update the description again. I'd like to wrap up because it's been pretty thoroughly explored.
That sounds right to me since @kRITZCREEK's example would probably be aliased anyway. Does the period have any semantic meaning besides an allowed character in the module name? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 12, 2016
Rewrote the description with @garyb's EBNF and explanation (#1901 (comment)) and @tfausak's proposal (#1901 (comment)).
texastoland
commented
May 12, 2016
|
Rewrote the description with @garyb's EBNF and explanation (#1901 (comment)) and @tfausak's proposal (#1901 (comment)). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
@paf31 in #2012 (comment): I think that it's perhaps a little hasty to follow Elm's lead on imports
Elm actually felt quite inconsistent story about imports when I tried to build the same table presented above and on the Haskell wiki. The only suggestions are to make the most common use cases the easiest to type and less confusing (open imports can cause warnings and it took me a bit to understand why import Prelude as Prelude isn't redundant). Essentially designing for least surprise.
a good default [in Elm] needs to differentiate
List.mapfromSignal.map
It goes beyond Elm's lack of higher kinded polymorphism to their naming in general. Are your suggesting qualified imports are discouraged?
I'm also not a fan of the suggestion to bring names into scope using the last segment of the module name.
Because YAGNI
? Because the period is just convention so too magical? On the other hand it's the only way I've seen qualified imports used besides the first letter of the last segment (which seems worst of all). Ultimately I incorporated @tfausak's suggestion because of lack of feedback.
TL;DR
Do any of the 3 changes listed in the description have consensus?
texastoland
commented
May 13, 2016
•
Elm actually felt quite inconsistent story about imports when I tried to build the same table presented above and on the Haskell wiki. The only suggestions are to make the most common use cases the easiest to type and less confusing (open imports can cause warnings and it took me a bit to understand why
It goes beyond Elm's lack of higher kinded polymorphism to their naming in general. Are your suggesting qualified imports are discouraged?
Because TL;DRDo any of the 3 changes listed in the description have consensus? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
nwolverson
May 13, 2016
Contributor
import X.Y meaning import X.Y as Y seems like a bad idea. "I think changing the default qualified import would encourage people to put something unique at the end" - but for example, Control.Monad.Eff.Class, all these .Class modules - I don't think it's desirable to encourage renaming all these.
|
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jplatte
May 13, 2016
Contributor
I had a look at some of the modules on pursuit and I think it would actually work reasonably well. I also like the idea in general and think using Eff.Eff would be a thing I might actually do, while I would never want to write Control.Monad.Eff.Eff. However @nwolverson is right in that existing naming conventions are a problem in some places here. There are more examples:
Data.Array.ST<anything>.Lazy<anything>.Types<anything>.Unsafe
If we want to make it a convention to have the part after the last . be the default qualified name, we should at least have a good recommendation of an alternate naming scheme. And of course we'd break a lot more code by renaming what seems like at least 5% of the modules on Pursuit.
|
I had a look at some of the modules on pursuit and I think it would actually work reasonably well. I also like the idea in general and think using
If we want to make it a convention to have the part after the last |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
we should at least have a good recommendation of an alternate naming scheme.
My goal is to not change that much.
Control.Monad.Eff.Class, all these.Classmodules - I don't think it's desirable to encourage renaming all these.
The corollary would be to conjecture that not doing so would encourage people to name modules Neon or Batteries (@tfausak only does so for top level exports) which might be worse. His suggestion was motivated that I never import a module as its fully qualified name (except Prelude) whereas I do reuse the last segment. In the case of .Class modules there shouldn't be overlap collision layering with others e.g. Control.Monad.Aff.Class? @kRITZCREEK also pointed out specified imports (import X (A)) are more conventional anyway.
texastoland
commented
May 13, 2016
•
My goal is to not change that much.
The corollary would be to conjecture that not doing so would encourage people to name modules |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
@kRITZCREEK also pointed out specified imports (import X (A)) are more conventional anyway.
I'm not sure about that, I've actually started using more qualified imports recently and I prefer them to explicit ones in most cases. I think it's often easier to read, as you don't have to refer to the imports list as often, and also they mean you don't have to update the imports list as often either (which has a further bonus of decreasing noise in diffs).
I'm not sure about that, I've actually started using more qualified imports recently and I prefer them to explicit ones in most cases. I think it's often easier to read, as you don't have to refer to the imports list as often, and also they mean you don't have to update the imports list as often either (which has a further bonus of decreasing noise in diffs). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
you don't have to update the imports list as often either
That benefit occurred to me as well but I prefer how bare identifiers read like a sentence to me. How often do you deviate from the last segment
convention? It looks good in Pulp :)
texastoland
commented
May 13, 2016
•
That benefit occurred to me as well but I prefer how bare identifiers read like a sentence to me. How often do you deviate from the |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
kRITZCREEK
May 13, 2016
Member
@kRITZCREEK also pointed out specified imports (import X (A)) are more conventional anyway.
I'm not sure about that
I am, but it misses my statement anyway. I was saying that explicit imports and qualified imports are our two most common imports and thus should get the most concise syntax.
think it's often easier to read, as you don't have to refer to the imports list as often
When you're inside an editor psc-ide can give you that information. Dot overlaps with record access and all the module names are a lot of noise.
and also they mean you don't have to update the imports list as often either
With the compiler complaining on superfluous imports, making correction suggestions and psc-ide around I don't think that's really an argument anymore.
which has a further bonus of decreasing noise in diffs
Again with all the compiler warnings in place and CI being present almost everywhere you can't really get imports wrong so you shouldn't be bothered to read the import section.
I am, but it misses my statement anyway. I was saying that explicit imports and qualified imports are our two most common imports and thus should get the most concise syntax.
When you're inside an editor psc-ide can give you that information. Dot overlaps with record access and all the module names are a lot of noise.
With the compiler complaining on superfluous imports, making correction suggestions and psc-ide around I don't think that's really an argument anymore.
Again with all the compiler warnings in place and CI being present almost everywhere you can't really get imports wrong so you shouldn't be bothered to read the import section. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
There are lots of situations where you want to read code without an editor handy. Code reviews on GitHub, for instance. I don't want a language that is only ergonomic when you have fancy IDE plugins set up.
Lots of people are ignoring compiler warnings right now, especially the import warnings. In the Haskell world there's not a consensus about whether imports should always be either explicit or qualified, even. When I was talking about noise I wasn't really concerned with getting things wrong, anyway, rather the ease of reading diffs.
|
There are lots of situations where you want to read code without an editor handy. Code reviews on GitHub, for instance. I don't want a language that is only ergonomic when you have fancy IDE plugins set up. Lots of people are ignoring compiler warnings right now, especially the import warnings. In the Haskell world there's not a consensus about whether imports should always be either explicit or qualified, even. When I was talking about noise I wasn't really concerned with getting things wrong, anyway, rather the ease of reading diffs. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
I was saying that explicit imports and qualified imports are our two most common imports and thus should get the most concise syntax.
texastoland
commented
May 13, 2016
•
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
Re the "last segment" convention, I usually follow it, but I don't really like that proposal regardless. I think it's too non-obvious, and I think the effort of typing out as List is worth it for readability.
Sorry to harp on about packages again but I think we could achieve the same goal by including package names in module names; if import "purescript-lists" List gave you the same as what import qualified Data.List as List gave you now, then we wouldn't need to come up with this hierarchy of Data.*, Control.*, Control.Monad.*, etc, and we wouldn't be worrying about this "last segment" convention. IMO the current module hierarchy is noisy and annoying, and doesn't even work because we still have to worry about collisions! The compiler wouldn't need to know anything about packages other than that packages a) have names and b) are made up of a certain group of source files. I realise this is a big change but the issue of module name collisions seems totally avoidable to me, and I think it would be such a shame if we didn't manage to fix it while we can still break things.
|
Re the "last segment" convention, I usually follow it, but I don't really like that proposal regardless. I think it's too non-obvious, and I think the effort of typing out Sorry to harp on about packages again but I think we could achieve the same goal by including package names in module names; if |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tfausak
May 13, 2016
Contributor
the current module hierarchy is noisy and annoying
Hear, hear! My hidden agenda is to sidestep the module hierarchy. I've never understood what it's good for. And it's inconsistent. Why is it Control.Monad but Data.Functor? Data.Int but Numeric.Natural? Not to mention the complete oddballs like Text.Read.
In Elm, when you need to work with lists, you say import List.
Hear, hear! My hidden agenda is to sidestep the module hierarchy. I've never understood what it's good for. And it's inconsistent. Why is it In Elm, when you need to work with lists, you say |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
Re above: I would volunteer to do at least a large chunk of the (admittedly massive) amount of legwork to update the compiler, build tools, and core libraries if we were to go down such a route.
|
Re above: I would volunteer to do at least a large chunk of the (admittedly massive) amount of legwork to update the compiler, build tools, and core libraries if we were to go down such a route. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
if we were to go down such a route.
I agree the times it's come up that the compiler isn't aware of packages have been inconvenient. Worth a separate issue? I assumed it's that way for a good reason or else there are just bigger fish (hence small proposal).
texastoland
commented
May 13, 2016
•
I agree the times it's come up that the compiler isn't aware of packages have been inconvenient. Worth a separate issue? I assumed it's that way for a good reason or else there are just bigger fish (hence small proposal). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jplatte
May 13, 2016
Contributor
@hdgarrood First, big
@AppShipIt Agreed as well. I think it's a good idea to get some version of this proposal into 0.9 and worry about the even bigger changes afterwards. I would favor the version as it currently is at the top of this thread (with @tfausak's idea included) because it would reduce the amount of times I'd have to explicitly specify a name for a qualified import.
|
@hdgarrood First, big @AppShipIt Agreed as well. I think it's a good idea to get some version of this proposal into 0.9 and worry about the even bigger changes afterwards. I would favor the version as it currently is at the top of this thread (with @tfausak's idea included) because it would reduce the amount of times I'd have to explicitly specify a name for a qualified import. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
May 13, 2016
Member
I'm all for renaming core library modules, but I think we should wait for 0.10, and try to get 0.9 out the door soon.
As for package names in module names, what about just adopting the convention that purescript-halogen goes under the Halogen namespace, purescript-halogen-bootstrap goes under Halogen.Bootstrap etc., so we'd have things like
Lists,Lists.Lazy, ...Prelude,Prelude.Functor,Prelude.Monad, ...Node,Node.FS,Node.ReadLine, ...
That way, no collisions, and you always can guess what the top-level module for a file should be. Pulp could even emit a warning if it didn't find the top-level file with the name it expected.
|
I'm all for renaming core library modules, but I think we should wait for 0.10, and try to get 0.9 out the door soon. As for package names in module names, what about just adopting the convention that
That way, no collisions, and you always can guess what the top-level module for a file should be. Pulp could even emit a warning if it didn't find the top-level file with the name it expected. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
I agree, there's lots of good stuff in the 0.9 branch right now and I'd like to get it out the door soon too.
With the convention you're proposing, if I'm understanding correctly, I think purescript-halogen and purescript-halogen-bootstrap would both be allowed to define a module called Halogen.Bootstrap, which could be a problem, especially if these packages are maintained by different people. (Perhaps purescript-halogen-bootstrap could put everything under HalogenBootstrap instead?) It does seem that this convention would significantly decrease the risk of module name collisions, though, and also would make it a lot easier to work out which package a module came from, so I like this idea too.
|
I agree, there's lots of good stuff in the 0.9 branch right now and I'd like to get it out the door soon too. With the convention you're proposing, if I'm understanding correctly, I think |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
I agree, there's lots of good stuff in the 0.9 branch right now and I'd like to get it out the door soon too.
Same.
As for package names in module names, what about just adopting the convention
That sounds an order of magnitude less obvious than the last segment idea and wouldn't resolve weaknesses of not being package aware
texastoland
commented
May 13, 2016
•
Same.
That sounds an order of magnitude less obvious than the last segment idea and wouldn't resolve weaknesses of not being package aware |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
I don't feel strongly about opening a new issue or not. It might be good to try to summarise all the issues brought up here in one place, though, as this conversation is getting rather long.
|
I don't feel strongly about opening a new issue or not. It might be good to try to summarise all the issues brought up here in one place, though, as this conversation is getting rather long. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
I rewrote the description at the top succinctly yesterday borrowing primarily from @garyb's comments.
texastoland
commented
May 13, 2016
|
I rewrote the description at the top succinctly yesterday borrowing primarily from @garyb's comments. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hdgarrood
May 13, 2016
Contributor
I mean, I find it's often useful to take a step back and just try to write down the problems everyone has brought up, without mentioning any potential solutions.
|
I mean, I find it's often useful to take a step back and just try to write down the problems everyone has brought up, without mentioning any potential solutions. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
Each directly addresses a UX issue:
import X (..)⇠import Xbecause we discourage open imports (also unspecified qualified imports) for reasons discussed in #1869.import Control.Monad.X⇠import Control.Monad.X as Xbecause the latter is mechanical and in the case ofimport Prelude as Preludeunintuitive.import X hiding (f)would no longer exposeX.fbecause it could be achieved explicitly with an additionalimport X (f) as X.- Subsumed by #1511 for module header parsing.
I can imagine 1 & 2 being resolved better by package aware modules. But I think it's fundamentally a different issue and if I were to ask for that or a package manager for Christmas it'd be the latter
texastoland
commented
May 13, 2016
•
|
Each directly addresses a UX issue:
I can imagine 1 & 2 being resolved better by package aware modules. But I think it's fundamentally a different issue and if I were to ask for that or a package manager for Christmas it'd be the latter |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
May 13, 2016
Member
Sorry, but I really don't like the idea of bringing names into scope in the last component of a module name by default (2). It assumes a convention which is not adopted universally.
The issue seems to be that module names are too long generally, so using the full qualified name would be unwieldy, but I think the solution there is to shorten module names, not try to canonicalize them somehow.
I'm not really even convinced that qualified imports are the best default. They work well in some cases, but not in others.
|
Sorry, but I really don't like the idea of bringing names into scope in the last component of a module name by default (2). It assumes a convention which is not adopted universally. The issue seems to be that module names are too long generally, so using the full qualified name would be unwieldy, but I think the solution there is to shorten module names, not try to canonicalize them somehow. I'm not really even convinced that qualified imports are the best default. They work well in some cases, but not in others. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
I'm not really even convinced that qualified imports are the best default.
It just makes them easier not default. On the contrary it makes open imports more explicit so that qualified imports are just as nice as specified ones.
texastoland
commented
May 13, 2016
It just makes them easier not default. On the contrary it makes open imports more explicit so that qualified imports are just as nice as specified ones. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
May 13, 2016
Member
import Control.Monad.X ⇠ import Control.Monad.X as X
I would say that makes qualified imports the default.
I would say that makes qualified imports the default. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
In that sense completely open is the current default which nobody wants (original impetus for this proposal)? import Control.Monad.X (liftX) continues to mean the same thing.
texastoland
commented
May 13, 2016
•
|
In that sense completely open is the current default which nobody wants (original impetus for this proposal)? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tfausak
May 13, 2016
Contributor
I don't like using open imports for two reasons. One, maintaining the import list is tedious. Tools help with this, but you don't always have the tools available. And I'm not a fan of languages that are only nice to use with tools. Two, it is easier for me to read code with qualified imports. I don't have to wonder if some function or value was defined somewhere in the module I'm reading or if it was pulled in from outside.
I agree that module names should be shorter. I think encouraging packages to have the top-level module that matches the package name is a good way to go.
I think the current default is a bad one. As it is, import X as X, the default and easiest thing to type, is a warning. Changing that to mean import X as X makes the default more useable to me.
Another note on module name shortening: I typically do things like import Data.Text as Text, which would just be import Data.Text in my proposal. However I also do things like import Data.Text.IO as Text, which I would have to continue to do. I don't have a problem with that.
|
I don't like using open imports for two reasons. One, maintaining the import list is tedious. Tools help with this, but you don't always have the tools available. And I'm not a fan of languages that are only nice to use with tools. Two, it is easier for me to read code with qualified imports. I don't have to wonder if some function or value was defined somewhere in the module I'm reading or if it was pulled in from outside. I agree that module names should be shorter. I think encouraging packages to have the top-level module that matches the package name is a good way to go. I think the current default is a bad one. As it is, Another note on module name shortening: I typically do things like |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
paf31
May 13, 2016
Member
I think it makes a fine default as long as you heed the warnings and turn them into explicit or qualified imports at some point.
Qualified imports by default will make operators a nightmare, for a start.
Also, it's confusing to me that import X requires me to write X.foo, but when I write import X (foo) (which psc-ide will conveniently generate for me), that becomes invalid.
|
I think it makes a fine default as long as you heed the warnings and turn them into explicit or qualified imports at some point. Qualified imports by default will make operators a nightmare, for a start. Also, it's confusing to me that |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
May 13, 2016
Also, it's confusing to me that
import Xrequires me to writeX.foo, but when I write importX (foo)… that becomes invalid.
Yeah that's the inconsistency introduced. But no matter how modules are named the alternative is to repeat yourself in qualified imports (import Neon as Neon). Either way I'd prefer a decision even if that means waiting for more clever packages.
texastoland
commented
May 13, 2016
•
Yeah that's the inconsistency introduced. But no matter how modules are named the alternative is to repeat yourself in qualified imports ( |
texastoland
referenced this issue
Jul 18, 2016
Closed
Unspecified imports when imports multiple modules as the same alias #2227
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
texastoland
Jul 25, 2016
Conclusion: half this proposal was implemented for 0.9. The remainder although more concise would break consistency #1901 (comment) and sidestep an underlying issue with module naming #1901 (comment) rather than improve that #1901 (comment). Closing in favor of an alternative proposal with a fresh conversation thread. Thanks!
texastoland
commented
Jul 25, 2016
•
|
Conclusion: half this proposal was implemented for |
texastoland commentedFeb 28, 2016
•
edited
Edited 1 time
-
texastoland
edited May 15, 2016 (most recent)
Continued from #1869:
Changes
import X (..)⇠import X(to make open imports explicitly opt-in via previous PR)import Control.Monad.X⇠import Control.Monad.X as X(to make qualified imports easy via @tfausak)#2131 (comment)import X hiding (f)only exposesAand notX.AorX.f(via @gary)#1511import Xrequired to exposeX.Afully qualified (via @paf31)Grammar
In the notation above:
<>reference previous rules[]represent zero or one (?in regex){}represent zero or many (*in regex)|represents one or the other (same as regex);introduces a commentThe rule could technically have been written as
<import>but the short form is nuanced sugar for the long one:<identifiers>(hidingnegates the set or..subsumes it) into the<qualified name>(or else unqualified local scope).... Unlike long form the<qualified name>defaults to the trailing part of the module name (e.g.XinControl.Monad.X). Any<import short>can be rewritten as<import long>:import Control.Monad.X⇠import Control.Monad.X (..) as Ximport Control.Monad.X as Y⇠import Control.Monad.X (..) as YExamples
import Control.Monad.XX.A,X.fimport X (A)Aimport X (..)A,fimport X hiding (f)Aimport X as YY.AY.fimport X (A) as YY.Aimport X (A) as XX.Aimport X (..) as YY.A,Y.fimport Y hiding (f) as YY.A