Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upDisallow hyphens in Rust crate names #940
Conversation
lfairy
added some commits
Mar 4, 2015
This comment has been minimized.
This comment has been minimized.
|
For example. I may have a crate: extern crate "syncbox-io" as sioOnce there are dashes or underscores in the name, it's probably easier to alias to something shorter anyway for use in code. |
This comment has been minimized.
This comment has been minimized.
|
Big |
This comment has been minimized.
This comment has been minimized.
The readability/aesthetics argument is subjective, and not something everyone agrees on. The fact that all arguments for them come down to they look nice should sound at least a bit suspicious. Especially when there are more objective arguments on the other side.
They aren't, of course. But as @sfackler says, making this kind of renaming mandatory is not the way to go. It'll be yet another special case for newcomers to learn, and introductory tutorials to explain. Quite a few features of Rust -- C-style syntax, lifetime elision, Crate renaming will become optional sugar, as it should be.
Note that if we disallow hyphens, we'll be able to do this instead: extern crate syncbox_io as sio; // imaginary syntaxwhich compares favorably to use std::fmt::Write as WriteFmt;In fact, hyphens are the only character we allow in crates which aren't also valid in identifiers. Without them, we shouldn't need quotes around names at all. I'm not pushing for this syntax here, but such a change would be possible in the future. (And before someone brings up Go: in that language the string is a path to the library, not a crate name. Note that Go doesn't allow hyphens in crate/package names either.) |
This comment has been minimized.
This comment has been minimized.
|
+1
|
This comment has been minimized.
This comment has been minimized.
|
I'll admit that |
This comment has been minimized.
This comment has been minimized.
|
let's do this... |
This comment has been minimized.
This comment has been minimized.
jmesmon
commented
Mar 6, 2015
|
I suppose another alternate is to allow |
This comment has been minimized.
This comment has been minimized.
|
It's ambiguous - is |
This comment has been minimized.
This comment has been minimized.
jmesmon
commented
Mar 6, 2015
|
@sfackler sure, we'd also need to change parsing rules (that would be a single ident), but it is an alternative. |
This comment has been minimized.
This comment has been minimized.
|
To avoid ambiguity, whitespace would be needed (e.g. |
This comment has been minimized.
This comment has been minimized.
|
Is there any language that allows |
This comment has been minimized.
This comment has been minimized.
|
@tshepang Lisp-family languages do, but they usually require spaces between tokens as well. I don't know of a C or ML-style language that does that. |
This comment has been minimized.
This comment has been minimized.
|
@lfairy I forgot about Lisp, thanks. |
This comment has been minimized.
This comment has been minimized.
dtantsur
commented
Mar 6, 2015
|
+1, using rustc-serialize is a bit weird at first |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
I agree to @P1start's rationale. Encoding currently has both |
This comment has been minimized.
This comment has been minimized.
|
Big |
This comment has been minimized.
This comment has been minimized.
|
We talked about this RFC in today's meeting. One of the points in favor of using hyphens, which I don't think has been stressed enough, is that there is a large amount of precedent for using I am personally sympathetic with the hyphen convention, and I believe that going against the precedent-at-large to gain consistency is not worth it. I think that this RFC does a great job of laying out the alternatives (thanks!), and I would personally push for the fuzzy matching alternative to become the detailed design. Of the two motivations, consistency and usability, fuzzy matching would solve the usability problem. There are probably still some more details to hash out precisely what happens, but overall I think it's the best direction to go in. We can continue the trend of hyphens set by other communities without sacrificing usability. In this situation we would probably continue to retain the quoting syntax for renaming if necessary, it just wouldn't be necessary if you used the canonical "extern crate" name for a crate. |
This comment has been minimized.
This comment has been minimized.
|
I don't think fuzzy matching really solves the usability problem, it just
adds complexity.
|
This comment has been minimized.
This comment has been minimized.
|
@steveklabnik could you elaborate a little more on why you think it doesn't solve the usability problem? I'm curious what use cases you have in mind and what possible extensions could be added (not that they should be, just curious!). The specific use-case I have in mind is what's spelled out in the RFC: extern crate "rustc-serialize" as rustc_serialize;
// vs
extern crate rustc_serialize; |
This comment has been minimized.
This comment has been minimized.
reem
commented
Mar 10, 2015
|
-1 for similar reasons as @carllerche. Hyphenated names are often meant to be renamed. For instance, I usually do Another case is with distributing iron-related stuff, perhaps I want to distribute a set of crates under |
This comment has been minimized.
This comment has been minimized.
|
Without hypens, crate names are just identifiers. With hypens and fuzzy matching, now I have to explain why it's not just an identifier, and what those fuzzy matchers are, when instead, we could all just use identifiers and sidestep this whole mess. I guess that's the core of it: we can pile on rules to make hypens look just like underscores, or we can just use underscores and call it a day. It's not like there are a zillion packages right now, but I certainly would use a non-hyphenated package over a hyphenated one, if they provided simliar functionality. It's ugly hoop-jumping for no actual benefit, imho. |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@reem, I am thinking, would it be a good alternative to state in the guide lines that But then on second thought, I'd rather we permit @everyone, would the above be a good alternative? |
This comment has been minimized.
This comment has been minimized.
I definitely agree that adding any form of fuzzy matching would be adding more complexity. This RFC classifies today's behavior as both complex and hindering usability. I'd like to tease apart these concerns in this discussion. You previously said "I don't think fuzzy matching really solves the usability problem", but it sounds like you're mostly worried about the complexity a fuzzy match would be adding? To be clear, I'm not disagreeing that a fuzzy match adds complexity, only that a fuzzy match does not solve the usability problem. I think @sfackler put it well in that today a
I agree, and my motivation for pushing back on disallowing hyphens is that I like hyphens, not so much that I don't want to go migrate all the crates on crates.io :) |
This comment has been minimized.
This comment has been minimized.
I believe that scheme is similar to what Ruby employs, although I'm less sure about Python or NPM. It does clash, however, with the |
This comment has been minimized.
This comment has been minimized.
Valloric
commented
Mar 11, 2015
|
Please don't add name fuzzy matching! Better leave it as is than add a possible footgun to the language. The fuzzy matching idea is much too clever for what crate importing is supposed to be doing. Forcing everyone to just use the underscore is a better idea IMO, but damn-nigh everything is better than fuzzy matching the name. It's way too magic. |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton, we may fuzzy match Also, we may mimic how The mandatory renaming avoids name clashes nicely, without introducing the concept of package/crate namespacing into the compiler. (Besides |
This comment has been minimized.
This comment has been minimized.
|
Another thing to mention, https://github.com/rust-lang/rfcs/blob/master/text/0430-finalizing-naming-conventions.md specifically says use |
This comment has been minimized.
This comment has been minimized.
Yes and yes. I've reworded the section to clarify these points, thanks!
Great! |
This comment has been minimized.
This comment has been minimized.
|
If we are doing this (allow
(Note: macro packages are to be renamed, but I can also accept leaving them as an exception as "foo macros" flows natually, in contrast "foo sys" doesn't.) Also when we create a new package in cargo, I hope it can warn about names that may break convention, like |
This comment has been minimized.
This comment has been minimized.
|
@CloudiDust I agree that we need an official convention on this, but I think that should be done in a separate PR. |
This comment has been minimized.
This comment has been minimized.
ncm
commented
Mar 14, 2015
|
If there is a separate mapping between file and crate names is certainly doesn't need to be duplicated here. Mapping "-" and "_" still seems like a wart. If it is always a suffix anyway, maybe "-" could map to "::" in identifier-land? |
This comment has been minimized.
This comment has been minimized.
ncm
commented
Mar 14, 2015
|
Or, could a "-anything" suffix be simply omitted from the identifier? "-suffixes" don't seem to disambiguate anything. |
This comment has been minimized.
This comment has been minimized.
|
@ncm, @Ifairy @alexcrichton, what about retaining (Note, there are no quotes around The advantage is that it is very clear hyphens/slashes are namespacing sigils. (And hyphens have to be mapped anyway.) The disadvantage is |
This comment has been minimized.
This comment has been minimized.
arthurprs
commented
Mar 14, 2015
|
The current state of the RPC is reasonable, although I'd prefer to forbid "-" on both ends for the sake of consistency. Does the core team expect to make a decision on this next week? I mean, the schedule is tight for the beta release. |
This comment has been minimized.
This comment has been minimized.
|
@arthurprs, I agree forbidding @everyone, I suppose one of the reasons to retain hyphens in package names is to avoid massive renaming on crates.io. And, hyphens are used for namespacing already. I looked at crates.io and found that most packages I saw were (correctly) using While the convention has not been officially established, I'd be very surprised if Mapping hyphens to underscores has one important advantage: it is simple. But it has one disadvantage: the namespacing information is lost in source code. If all crates become So hyphens -> slashes feels natural to me. Note: |
This comment has been minimized.
This comment has been minimized.
jmesmon
commented
Mar 15, 2015
|
@CloudiDust which accepted RFC (or other documentation) specifies that Unless there is something, I view the discussion on namespacing crates as something that probably belongs in a separate RFC. |
This comment has been minimized.
This comment has been minimized.
|
@CloudiDust Another disadvantage would be inconsistency: this @arthurprs The RFC is written with a tight time frame in mind (all proposed changes are really easy to implement) so it shouldn't be an issue. @jmesmon I think you're reading to much into his comment. He already mentions that "the convention has not been officially established". |
This comment has been minimized.
This comment has been minimized.
|
@Ifairy, I'm afraid that mapping hyphens to underscores would tempt people to go "I import with underscores anyway, so I'll always name the packages with underscores only." But this may be a minor problem and I am worrying too much. Let's go with this RFC as written then. |
This comment has been minimized.
This comment has been minimized.
|
+1
|
This comment has been minimized.
This comment has been minimized.
|
We discussed this RFC today and the reception was quite favorable of the RFC as-written. I'm going to hold off on merging for a bit to allow any final comments, but otherwise I think this is good to go, thanks @lfairy! |
alexcrichton
self-assigned this
Mar 19, 2015
alexcrichton
referenced this pull request
Mar 19, 2015
Closed
Tweak how hyphens in crate names are allowed #23533
alexcrichton
merged commit 19bf9bb
into
rust-lang:master
Mar 19, 2015
This comment has been minimized.
This comment has been minimized.
|
The current version of this RFC appears to have widespread support now by striking a nice balance between consistency in the language while allowing usage of naming conventions in external functions such as Cargo and crates.io. I've now merged this RFC, and thanks again @lfairy! Tracking issue: rust-lang/rust#23533 |
fenhl
referenced this pull request
Mar 26, 2015
Merged
Update rustc-serialize import to new syntax #64
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton was the intention here to also forbid hyphens in the "crate name" of an output binary? It seems like we need to do something with our test suite to avoid lots of annoying warnings there. |
This comment has been minimized.
This comment has been minimized.
|
@pnkfelix currently it is invalid to manually specify |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton all I know is that when I currently compile tests by hand via the command lines I see from e.g.:
(maybe I should just file a bug then.) |
This comment has been minimized.
This comment has been minimized.
|
@pnkfelix are you sure you're rebased on the current master? I believe I removed that warning very recently. |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton yeah okay I don't see it now. |
This comment has been minimized.
This comment has been minimized.
gsingh93
commented
Apr 3, 2015
|
Now that quotes are removed, are crate names like static no longer valid? |
lfairy commentedMar 5, 2015
Disallow hyphens in Rust crate names, but continue allowing them in Cargo packages.
Rendered