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
Improve require
lookup
#2690
Comments
Tbh. I already dislike the current behavior, it's way too magical and I'm only waiting for conflicts to happen, in non obvious ways. Trying even more files will only make that ever more so likely. I would still prefer a basically Ruby style system that's easy to follow and understand. For |
But the current way, even though with more "magical" logic in the compiler, is simpler for the user. Just add dependencies, do Also in Ruby there's this problem, which we don't have, and we would have if we support modifying the load path, and not making the compiler scope requires to a directory. In this case, I think more power is less beneficial. |
My point is that I already don't follow the current logic. Ruby's system is easy to exercise in your mind and thus easy to debug. Yes, the current way is easier for the user, but only as long as it works perfectly. Have a conflict and need to figure where/how/when/why? Welcome to debugging hell following all this logic. Then welcome again for finding a workaround taking all that complex logic into account. |
It's explained here. I think it's pretty simple:
Is there are way to make that simpler? Next, we need to allow requiring just some files inside a shard. This logic can be explained in one paragraph, without mentioning CRYSTAL_PATH, how shards modifies it (because it doesn't), and users don't even need to know how it all works, they just do Anyway, that's just my opinion :-) |
Alright, just forgot a rule when repeating it the second time. I guess it makes no difference if you forget or don't know 2 of 3 or 1 of 3 instead of 1 of 2. Or none of one because there's only one and you know there has to be one. With the current rules a |
To src or not to src, it should be transparent to the user and they shouldn't care about how shards are expanded in lib folder as long as the usecases Maybe the more complicated story is how to create a shard in the app that you might want to extract later. This last user story might add some additional criteria. Also, we can make the compiler give some information how each require is resolved... |
@jhass I don't follow, there's no way one shard can shadow another shard
As you can see, the first component in the require always scopes the search. So one shard can't pollute another namespace. Expanding to "foo/src/foo" is only applied for the first component. |
The whole I did that for minitest.cr as a matter of habit (it was my first shard), but I'll eventually put everything under I think this is simpler and more solid than perpetuating the Rubygem issue. We already don't version the installed shards (because we deal with project specific dependencies, not a central install repository). We already don't push each shard On another topic: we should indeed change the way shards installs dependencies to extract the shard's root as
Where only the first part, |
I like @ysbaddaden idea, but this either don't solves what people in google groups where looking for or change how shards were structured until today. As an example, using a
to require src/foo/bar.cr you have to still write foo twice:
to require version and bar separetedly like this:
|
Indeed, |
Doesn't that give up namespace -> filename mapping conventions? |
As someone newer to Crystal, the behavior @asterite has described seems to be the method of least-surprise, for what it's worth. I think this matches up with the behavior described by @ysbaddaden in the previous comment, in terms of file loading logic (if I understand his comment correctly). Separating load logic from directory structure would make the structure issue more of a developer preference. Personally, I would prefer the nested structure, but that is mainly due to the fact I'm coming from Ruby, whether that is good or bad. The flexibility to allow both structures would probably be well received, especially for people coming from other language backgrounds. |
@ysbaddaden @cjgajard I think the last point iv in this comment has a mistake. Shouldn't it be:
In fact, for the last part I'd say it should be:
|
That is, a shard should keep the |
Personally, I like |
@cjgajard For the standard library
with more files inside |
@asterite Sorry I was not clear explaning my point, is not about requiring the standard library that's already covered.
vs
EDIT: I would like to set that I think For iii you already said requiring |
@asterite dropping the namespace under The iv. example is because we support |
@cjgajard having to put everything under |
@cjgajard I understand what you mean now. When we were designing how We then though "Hm, since we have that rule, we could (ab)use it in the standard library", and so there's no So, we can make
We don't search for @ysbaddaden We then have to decide how to resolve So, we can make
However, it's a bit odd that then doing Alternatively, we can make
I don't think 2 and 3 conflict, it should not be common to have those two files. We can adopt the non-namespaced structure, making Just to clarify,
That is, only the first component But what should we do? Should we encourage a namespaced structure inside shards, or a non-namespaced structure? |
Seems
|
Another thing to keep in mind while we're touching this is #321 |
@jhass Yes, I'd like to do that too. I think that's just a matter of adding both @DougEverly I'd like to keep require simple. To require relative to the current file one now has to do |
@jhass what's your thought on namespacing vs. not namespacing? |
Albeit not a super strong one I have a tendency towards doing it, never liked the |
@jhass I mean, should we layout things like:
Or like:
|
I understood that, hence my last sentence. |
Oh, I misread you. Well, for now we can support both forms and leave that decision for later. |
I use both styles.
Oh, I always use a "src/"-dir of course! So, in short: I really like the proposed steps of look up. @jhass makes a good point regarding the shadowing, which could happen when refactoring, and hitting save in an editor view pointing to old location or so), I think therefore all the locations should be checked for each require, and if found in more than one of the places: error! ( |
Oh, and |
Reference: https://groups.google.com/forum/?fromgroups#!topic/crystal-lang/eJ9Ijb54Tnk and https://groups.google.com/forum/?fromgroups#!topic/crystal-lang/uy-jxSp-b7U
TL;DR: to require a file "specific" inside a shard "foo" one has to do
require "foo/foo/specific"
. It would be nice to just have to dorequire "foo/specific"
.The lookup rules should probably change to this:
require "foo"
, search "foo.cr" and "foo/foo.cr" in the path (this already works like that)require "foo/bar"
, search "foo/foo/bar.cr" in the path (I think searching "foo/bar.cr" is not needed)As a separate concern, shards right now puts only the "src" directory inside "libs". Maybe it would be simpler/easier to put the whole repo inside "libs", inside a folder with the shard name. Then the require lookup should change to:
require "foo"
, search "foo.cr", "foo/foo.cr" and "foo/src/foo.cr" in the pathrequire "foo/bar"
, search "foo/foo/bar.cr" and "foo/src/foo/bar.cr" in the pathThis has the additional benefit that if a file inside "src" requires something outside "src" (maybe a "data" directory"), it will be available, otherwise it won't be found. Example
And maybe this last point simplifies shards? I don't know.
Finally, we should maybe rename "libs" to "lib": #321
/cc @ysbaddaden @jhass
The text was updated successfully, but these errors were encountered: