Show the actual value of constant values in the documentation#66221
Show the actual value of constant values in the documentation#66221bors merged 1 commit intorust-lang:masterfrom
Conversation
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
fb581b9 to
deb4f7a
Compare
|
I'm not sure we want this... Why not keeping the original value and putting it into a link to its definition instead? |
|
The motivating example here is I saw the tweet and it also happened to me multiple times so I figured I might as well fix it 🙂 Prior art: |
|
☔ The latest upstream changes (presumably #66233) made this pull request unmergeable. Please resolve the merge conflicts. |
216cfc6 to
b14632f
Compare
|
So in #53409 the problem seems to be over-detail when displaying a module, and this view it's not changed by this PR at all. For #42759, it's at least somewhat of an improvement because for non-scalar values we will mirror the source. However clearly the author of the issue wanted to see a value for the constant, so not showing anything is kind of a funny fix. #32735 is an ugly corner case, but I'll argue that there are more "should show the value of" than "should hide the value of" |
|
I still have issue with this PR: in case it is a mathematical value like PI, why would I want the full number and not the constant PI? const stuff: f64 = math::PI;
const stuff: f64 = 3.1415;I strongly prefer the first case. Even more considering that if you really want to see the full number, you can just click on the value... |
|
What if we showed it like this (approximated html): Where the value part is only added for scalars? While remaining authentic to the original source code, it'll also solve the problem for |
|
Providing the "complete" value as a comment would be fine for me. For example: |
ed5ef16 to
7b9ef3f
Compare
|
@GuillaumeGomez what do you think about this? It's a little noisy for some values (like |
7b9ef3f to
d2d4429
Compare
|
In the first case, since this is already a value, there is no need to add the comment. Can you not provide it in case this is "direct" value? (no idea how to express correctly what I have in mind, I hope people will understand XD) |
6a89be2 to
5062d47
Compare
|
I changed the code to not show the computed value for consts which are literals (or I implemented a version (diff) which adds the |
5062d47 to
de2ff01
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
de2ff01 to
4200924
Compare
|
Instead of using HIR to check, can't you compare both displays and if they're the same, not print the comment one? |
|
This is what the current code does, but it doesn't catch everything (like |
|
Well I guess you can limit the display to nth first characters or something along the line? But yes, I think we're close to the end. Thanks so much for working on this! |
|
@GuillaumeGomez I added splitting for big numbers! Let me know what you think. |
|
@ohadravid Do you have screenshots by any chance? :) |
|
☔ The latest upstream changes (presumably #66984) made this pull request unmergeable. Please resolve the merge conflicts. |
8f2fd28 to
7a10e20
Compare
|
☔ The latest upstream changes (presumably #67202) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Nice! Well then, unless @ollie27 wants something else to be updated, you can r=me once you have rebased. |
7a10e20 to
ea7b622
Compare
|
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
ea7b622 to
53ff21e
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
53ff21e to
e398f59
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
e398f59 to
531622a
Compare
531622a to
811bdee
Compare
|
ping @ollie27? 🙏 |
|
@bors r+ rollup |
|
📌 Commit 811bdee has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
Show the actual value of constant values in the documentation Fixes #66099, making rustdoc show evaluated constant scalar values.  where `main.rs` is ``` pub const VAL3: i32 = i32::max_value(); pub const VAL4: i32 = i32::max_value() - 1; ``` As a fallback, when a constant value is not evaluated (either because of an error or because it isn't a scalar), the original expression is used for consistency. I mimicked the way min/max values of integers are [`pretty_print`ed](https://github.com/rust-lang/rust/blob/master/src/librustc/ty/print/pretty.rs#L900), to show both the value a the "hint". While a little goofy for `std`, in user crates I think it's actually rather helpful.
|
☀️ Test successful - checks-azure |


Fixes #66099, making rustdoc show evaluated constant scalar values.
where
main.rsisAs a fallback, when a constant value is not evaluated (either because of an error or because it isn't a scalar), the original expression is used for consistency.
I mimicked the way min/max values of integers are
pretty_printed, to show both the value a the "hint". While a little goofy forstd, in user crates I think it's actually rather helpful.