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

Display "edition" from manifest on crate-page #1541

Closed
lukaslueg opened this issue Oct 26, 2018 · 13 comments · Fixed by #9997
Closed

Display "edition" from manifest on crate-page #1541

lukaslueg opened this issue Oct 26, 2018 · 13 comments · Fixed by #9997
Labels
C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@lukaslueg
Copy link
Contributor

This is an example of a crate that has an explicit edition = "2018" in it's manifest. It would be nice if the crate-page would show a marker/banner/hint that this crate expects a 2018-environment

@Mark-Simulacrum
Copy link
Member

This just requires a recent enough compiler so I don't think this is something we should do this as it's really no different than "this crate requires 1.31" which is a separate discussion (there have been multiple RFCs on that)

@carols10cents
Copy link
Member

@lukaslueg and @Eh2406, could you elaborate a bit on why this would be nice/useful and different from the compiler version the crate requires?

Given a new enough compiler, you can depend on crates that use either edition without needing to care which edition the dependency uses, barring some potential keywords used as functions in the public API that would need the raw identifier syntax.

Or do you mean as a potential contributor to the crate, rather than a potential user of it?

In other words, if this information was displayed on crates.io, what would you do differently based on it?

@Eh2406
Copy link
Contributor

Eh2406 commented Oct 30, 2018

I had not fully thought this thure when I emoted, having read the responses I agree that it is just a weaker form of MSRV.

@lukaslueg
Copy link
Contributor Author

Well, this is actually a weak argument in the first place: While there is an ongoing discussion on how to communicate rustc version requirements, the edition-key in the manifest is as clear as it could: Either it's there (because you have to have it in order to use 2018) or it's not there (which gets us "needs rust 1.28ish"). Since it's also readily available from the manifest, it's very easy to implement without any kind of ambiguity. Besides - and the arguments get even weaker here - the whole edition concept is probably less known to the wider audience than one may think. A hint on crates.io could help communicate "hey, this is a thing!"

@meven
Copy link

meven commented Dec 28, 2018

I think it would make sense, for the users to know about editions as @lukaslueg mentioned and to advertise the lattest edition (in case someone missed the information) and slightly encourage maintainers to migrate to a more recent edition because it would grant their crate the edtion 2018 badge.
But also because a byproduct of this could be to have statistics about the number of "edtion 2018" crates and crates "edtion 2015".
It would also add coherence with other tools such as the playground.

@kornelski
Copy link
Contributor

kornelski commented Jan 9, 2019

FWIW I've implemented that on lib.rs: https://lib.rs/crates/tokio-async-await (edit: shown only when the edition is very new or very old, otherwise it was too noisy).

screenshot 2019-01-09 at 15 46 45

@Turbo87
Copy link
Member

Turbo87 commented Feb 12, 2024

Since this issue was opened we've started to parse the embedded Cargo.toml files ourselves. For example, we are reading the rust-version value from the manifest, and are saving it to the database for each uploaded version (see

let rust_version = package.rust_version.map(|rv| rv.as_local().unwrap());
).

Doing something similar for the edition key in the manifest should be relatively straight-forward by essentially copying what we already have for the rust-version field.

We could then display it like this:

Bildschirmfoto 2024-02-12 um 13 20 28

@epage
Copy link

epage commented Feb 12, 2024

For myself, I feel like screen real estate is precious: the more you show, the harder it is to use what you've got.

Edition is an implementation detail. It should not matter to cargo add or cargo install. Someone mentioned "advertising" new Editions but that feeds week to justify more space being used. However, it is one of many fields that can be used to statically infer an "at earliest" MSRV if one is not set. We could infer an MSRV but we'd need to be clear that it is not the true MSRV (inferring has limits). We could say showing Edition when MSRV is unknown could be a good hint but then users are having to try to translate in there head and makes it harder to scan MSRVs.

@Turbo87
Copy link
Member

Turbo87 commented Feb 12, 2024

in any case we will want the information in the database at least. how exactly we display it or what we derive from it we can still discuss once we actually have the information :)

@epage
Copy link

epage commented Feb 12, 2024

I'm curious, what is helped by having it in the database?

@Turbo87
Copy link
Member

Turbo87 commented Feb 12, 2024

at the very least it will make it easier to run edition data analysis on the database dump, but as you wrote above, inferring an MSRV from the edition field is also a possibility

@epage
Copy link

epage commented Nov 19, 2024

So last this was discussed, the next step was to put it in the database "for reasons" without an end plan for this. Now a PR is merged to show it without any reasoning given in the PR. Yes, its at least in a tooltip so its not taking up space but being tied tg the MSRV tooltip and saying the package requires an Edition at best does nothing (they already have an MSRV) and at worse implies some additionally requirement beyond MSRV.

@Turbo87
Copy link
Member

Turbo87 commented Nov 19, 2024

some people find it informational and I don't see why we should completely hide the information. if you are not one of those people then that's fine, but I don't see what's wrong with showing this information in the tooltip, since, as you requested above, it's not even using up the precious real estate on the screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants